Store filter other_shapes as one ETS row per shape - #4785
Open
alco wants to merge 1 commit into
Open
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4785 +/- ##
=======================================
Coverage 60.05% 60.06%
=======================================
Files 397 397
Lines 43772 43772
Branches 12590 12590
=======================================
+ Hits 26289 26290 +1
Misses 17401 17401
+ Partials 82 81 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e97bd849f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
WhereCondition kept a node's non-indexable shapes in a single map inside
the node's where_cond_table row. Every add or remove looked the whole map
up, modified it and inserted it back, copying every where clause on the
node in and out of ETS each time. Adding a shape was therefore O(n) in the
number of non-indexed shapes already on the node and (re)building the
filter — including restoring all shapes on startup — was O(n²). Adding an
indexed shape to such a node paid the same cost because the index_keys
bookkeeping lived in the same row.
Non-indexed shapes now live in a dedicated :ordered_set table with one row
per shape keyed {condition_id, shape_id, branch_key}, so insert and delete
are O(1), and routing/emptiness checks use key-prefix matches that only
traverse the node's own rows. The where_cond_table row holds just the
node's index_keys.
Measured on one machine, per-shape add cost with 5,000 non-indexed shapes
on a node went from ~1.9 ms to ~2 µs; routing latency is unchanged.
Fixes #4743
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FfoVxddTs2Y2nZewNozbYR
alco
force-pushed
the
fix/other-shapes-per-row-ets
branch
from
September 1, 2026 09:30
9e97bd8 to
63fa1df
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4743
Problem
WhereConditionkept a node's non-indexable shapes (other_shapes) as a single map inside the node'swhere_cond_tablerow. Every add or remove did lookup →Map.put/Map.delete→ insert, copying every where clause on the node in and out of ETS each time. So:ShapeLogCollectorrestores shapes on startup — was O(n²);index_keysbookkeeping lived in the same row and was rewritten through the same lookup/insert.The copy is proportional to the total size of the clause ASTs on the node, so shapes with large residual clauses (e.g. an
IN (...)list) make it worse.Fix
Non-indexed shapes now live in a dedicated
:ordered_settable with one row per shape, keyed{condition_id, shape_id, branch_key}. Insert and delete are single-row operations, and routing /all_shape_ids/ the node-emptiness check use key-prefix matches withcondition_idbound, which an:ordered_settraverses as a range rather than a full scan (same techniqueSubqueryIndexalready uses). Thewhere_cond_tablerow is reduced to{condition_id, index_keys}.Numbers
Per-shape
Filter.add_shapecost on one machine, n = non-indexed shapes already on the node, K = size of anINlist in the clause:number > iid IN (K=10) AND number > iid IN (K=150) AND number > iRemoval shows the same change.
affected_shapeslatency on such nodes is unchanged within noise — that path is dominated by evaluating each clause, not by the ETS read.Independent of #4742, which reduces how many shapes end up on this path in the first place; the two compound.
🤖 Generated with Claude Code
https://claude.ai/code/session_01FfoVxddTs2Y2nZewNozbYR