test(querier): rfc0044.7/.8 pruning fixtures; unwrap the eq arm (slice 4) - #674
Conversation
…e 4) Partitioned scenarios pin the pruning contract: a non-candidate partition is skipped while results stay complete (.7), and a literal matching no template and no stored body scans zero row groups (.8). Writing .7 exposed that IS TRUE on the eq arm defeats DataFusion's pruning analysis — and it was never needed there: under a filter NULL and false are equivalent, so equality is total without it. The wrap stays where NULL genuinely differs from false: the != branch's IS NOT TRUE. Rationale inline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qtny6z6cA74xPZa4qRhk4F Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qtny6z6cA74xPZa4qRhk4F Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🟢 Ready to approve
The functional change is small, well-rationalized, and reinforced by new integration tests; only a minor doc-comment/assertion mismatch remains.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR advances RFC 0044 slice 4 by strengthening integration-test coverage for template-aware body == and by adjusting the compiler’s equality predicate to preserve DataFusion row-group pruning behavior.
Changes:
- Add RFC0044.7 and RFC0044.8 (full) integration tests asserting cross-partition pruning behavior and “cheap empty” behavior.
- Remove the
IS TRUEwrapper from the==template arm inbody_equalityto avoid defeating DataFusion pruning analysis, while keeping!=totalization viaIS NOT TRUE.
File summaries
| File | Description |
|---|---|
| crates/ourios-querier/tests/it/rfc0044_body_equality.rs | Adds RFC0044.7/.8 pruning-focused fixtures and assertions on scanned/pruned row groups. |
| crates/ourios-querier/src/compile.rs | Adjusts body == compilation to avoid IS TRUE so pruning can engage; keeps != semantics via IS NOT TRUE. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Low
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qtny6z6cA74xPZa4qRhk4F Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
Summary
RFC 0044 slice 4 — the pruning half of the contract:
IS TRUEon the eq arm defeated DataFusion's pruning analysis. It was also unnecessary there — under a filter, NULL and false are equivalent (both drop the row), so equality is total without it. The totalisation stays exactly where three-valued logic genuinely bites:!='sIS NOT TRUE. Rationale inline in the compile.10/10 rfc0044 scenarios, 127 lib tests, fmt/clippy clean.
Remaining: the RFC0044.9 property suite, then the green flip.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Qtny6z6cA74xPZa4qRhk4F