Repository navigation
quack: intersect ANDed row ranges on one column before lowering (D-TELEPORT-1) - #1487
Conversation
D-TELEPORT-1. fold_ranges runs at the top of lower_with, so lower,
lower_fused and lower_group_by all see it. Inside each AND, the direct
children Cmp(col, Range{lo, hi}) sharing a col become one range
[max(lo), max(max(lo), min(hi))), kept at the first one's position.
- Empty intersections stay valid empty ranges (lo == hi), never lo > hi.
- Row-order identity: only ranges on the same col merge; col is the only
order identity a Filter carries.
- Siblings of one AND only; never across OR or NOT; nested ANDs are not
flattened.
- A range with lo > hi is not merged, so mask-risc still refuses it. A
bound past n_rows on the side the intersection drops cannot be seen
here; the producers that bound ranges never emit one. Documented.
Under COUNT/ANY the single range takes mask-risc's closed form (hi - lo,
no row visited). Under other terminals the survivors are still visited,
minus two passes and two slots.
Tests (range_fold_tests): one op and Lowering::Range for an overlapping
pair; an empty range for a disjoint pair; different columns, OR and NOT
left alone; an inverted range not merged; 400 seeded conjunctions with
planes and lane compares equal to a row-by-row oracle under both
lowerings. Disable runs, each red: hook removed; merge across columns;
min instead of max; no empty clamp; inverted merged; OR merged.
quack full suite and lance-graph-cypher-quack green.
Board: D-TELEPORT-1 row; D-TELEPORT-0 and D-SQLP-2 marked shipped (#1484).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015JH7YkXs5us6g3tG8NiFfP
|
Warning Review limit reachedYour organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Next included review available in 1 minute. View limit details
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: f17feba68b
ℹ️ 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".
| let p = fold_ranges(p); | ||
| if let Filter::Cmp(col, Cmp::Range { lo, hi }) = p { | ||
| if lo <= hi { | ||
| let seen = first.iter().find(|(c, _)| *c == col).map(|&(_, i)| i); |
There was a problem hiding this comment.
When an AND contains k valid ranges on distinct columns, first grows to k entries and every child scans it linearly, making lower and lower_fused take Θ(k²) time before gate_walk; because Col spans the full u16 range and Filter::and accepts arbitrary iterators, generated wide predicates can trigger billions of comparisons. Indexing by Col with a fixed table or map preserves linear planning time, and a wide distinct-column benchmark should cover this performance-sensitive path.
AGENTS.md reference: AGENTS.md:L24-L27
Useful? React with 👍 / 👎.
64 populations x 1,048,576 addresses in four regimes (sparse, dense, clustered, 256 archetypes). Compares quack-fused and in-place plane lowering, single-range extents, per-address u64 signatures and signature-equivalence classes on COUNT, Any, SUM and Rows; the canonical expression plus 20 random ones equal the oracle in every arm. Also measures transpose, class build, 1,000-address updates and spill/reload. Bit-sliced planes answer in ~0.03-0.05 ms per query regardless of data; signatures lose by 250-600x; classes win only at a few hundred classes and cost 30-100x more per update. Board: entries for D-TELEPORT-2 and D-PHOTO-0, D-TELEPORT-1 marked shipped (#1487). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015JH7YkXs5us6g3tG8NiFfP
2 participants
What
Adds a small rewrite,
fold_ranges, inlance-graph-quack. It runs first thing inlower_with, solower,lower_fusedandlower_group_byall pick it up.Inside each
AND, the direct childrenCmp(col, Range { lo, hi })that share acolare replaced by one range,[max(lo), max(max(lo), min(hi))), placed where the first of them was.Why
PR #1484's probe measured
Range AND Rangelowering to three passes (two range writes and anAnd, ~10 µs on 1M rows). After this rewrite the two ranges are already one before execution: a property of two boundaries.hi − lowithout visiting any row (Lowering::Range).Edges
Range { lo: x, hi: x }. This is still a valid range (lo ≤ hi), so mask-risc accepts it.Rangeis row ordinals in the order of the lane it was bound against, andcolis the only order identity aFiltercarries. So only ranges on the same column merge.ANDmerge. A range under anORorNOTis never merged with one outside it, and nestedANDs are not flattened.lo > hiis not merged, so mask-risc still refuses it.n_rowscannot be seen here, because a filter carries no row count. If it is on the side the intersection drops, the merged range runs.prefix_facetandaperture_facet, which bound ranges against a sealed lane, never emit one. This is written in the function's doc.Tests (
range_fold_tests)Lowering::Range, and gives the right count under both lowerings.ORandNOTare left alone, and a lone range is unchanged.lo > hi) is not merged.Disable runs, each red: hook removed; merging across columns;
mininstead ofmaxforlo; no empty clamp; inverted ranges merged; ranges merged underOR.Checks
lance-graph-quackfull suite passes; clippy-D warningsand fmt cleanlance-graph-cypher-quackpassesBoard: new D-TELEPORT-1 row; D-TELEPORT-0 and D-SQLP-2 marked shipped in #1484.
🤖 Generated with Claude Code
https://claude.ai/code/session_015JH7YkXs5us6g3tG8NiFfP
Generated by Claude Code