perf(querier): RFC 0031 — elide the count scan when materialization is complete - #485
Conversation
…s complete Comparative run #8 showed every limited query pays for the same row groups twice: a count/pruning scan plus the materialize scan. When the materialized result comes back under the limit it is provably complete, so the count is the returned row count and the count scan is pure redundant IO — exactly the common case for the RFC 0031 harness, whose completeness guard requires rendered == matched. Design: an additive opt-in (`QueryOptions` + `Querier::run_query_with`; `run_query` delegates with the default) rather than a default-behavior change, because the RFC 0017 §3.4 contract pins a limited query's `stats` equal to a count-only query's (rfc0017_6) and the RFC 0016 metrics consume that shape — an elided run cannot satisfy the bytes equality. Under the opt-in the execution is materialize-first with a truncation fallback: - returned < limit ⇒ complete: rows = returned, count scan skipped. `stats` keeps the count scan's row-group pruning counts — the materialize plan prunes by the same predicate over the same file set, and an unreached limit never stops the scan early, so the counts are identical (asserted against a count-only run in the tests) — with bytes_read = 0, honest because the count scan genuinely never ran. - returned == limit ⇒ possibly truncated: fall back to the count scan, byte-identical to today's two-pass behavior (rows stays the full matching total; the pinned stats equality holds). The RFC 0031 harness (`ourios_query_answer`) opts in, so its measured total is what one answer-delivering query actually reads; its `count_scan_bytes` component is 0 whenever the scan was elided (the harness's own success path, bar the exact-limit edge, which the bench test now also covers). The bench spec test `honest_total_bytes_breaks_down_additively` is updated to the directed single-pass accounting (count_scan_bytes > 0 → == 0 on the complete path) — a deliberate contract change to the harness channel; the querier-side pinned tests are untouched. Measured locally (total bytes_read = count + materialize + registry): - comparative fixture (3 rows, `severity >= 0 | limit 1000`): 9,886 → 9,720 B (−166 B count scan) - synthetic 50,500-row store, selective 500-match `severity >= error | limit 1000`: 132,288 → 74,297 B (−43.8%) - same store, broad `severity >= 0 | limit 60000`: 593,572 → 314,946 B (−46.9%) New spec tests (crates/ourios-querier/tests/it/rfc0031_single_pass.rs) cover complete / truncated / exactly-at-limit / empty results plus the default two-pass shape staying intact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y
|
Warning Review limit reached
Next review available in: 48 minutes 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 (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Pull request overview
This PR introduces an opt-in single-pass execution mode in ourios-querier (RFC 0031 §3.6) that can skip the redundant count/pruning scan for complete limited queries by materializing rows first and deriving rows from the returned record count. The default execution path remains byte-identical by routing existing callers through QueryOptions::default().
Changes:
- Add
QueryOptions { elide_count_scan }andQuerier::run_query_with(...)as an additive API surface for opt-in execution behavior. - Implement materialize-first execution for limited queries under the opt-in, eliding the count scan when
returned < limitand falling back whenreturned == limit. - Update the comparative bench harness and add integration tests pinning the single-pass contracts (elision, fallback, exact-limit, empty-match, and default two-pass shape).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/ourios-querier/src/lib.rs | Adds QueryOptions + run_query_with, and implements single-pass (materialize-first) execution with count-scan elision when completeness is provable. |
| crates/ourios-querier/tests/it/rfc0031_single_pass.rs | New integration tests pinning elision vs fallback behavior and stats/bytes accounting under the opt-in. |
| crates/ourios-querier/tests/it/main.rs | Wires the new RFC 0031 integration test module into the consolidated test harness. |
| crates/ourios-bench/src/comparative.rs | Switches the comparative harness to run_query_with(..., QueryOptions::single_pass()) and updates the bytes-read contract tests accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
What
Single-pass execution for limited queries (RFC 0031 lever 2): a new additive
QueryOptions { elide_count_scan }+Querier::run_query_with(...). Under the opt-in, execution is materialize-first: when the returned rows number fewer than thelimit, the result is provably complete —rowsis the returned count and the count/pruning scan (which would re-read the same row groups for information already in hand) is skipped.returned == limit(possibly truncated) falls back to the count scan, sorowsis the full matching total in every case.Contract preservation
run_querydelegates withQueryOptions::default()— every existing caller (server HTTP, MCP, all tests) is byte-identical. The pinned spec testrfc0017_6_typed_row_payload_returned_b1b2_compatible(limited.stats == counted.stats) is untouched and passes; that pinned shape is exactly why elision is opt-in rather than unconditional.stats.row_groups_scanned/prunedstill carry the count-scan values — the materialize plan prunes by the same predicate over the same file set, and an unreached limit never terminates the scan early. This equality is asserted against a count-only run in the tests, not assumed.stats.bytes_read = 0is honest: that scan genuinely never ran; the three-component sum stays the total IO.honest_total_bytes_breaks_down_additivelyassertedcount_scan_bytes > 0— written for the two-pass harness. Wiring the harness to single-pass necessarily flips that assertion; the test now specifies the new contract and gained the exact-limit fallback case (limit == matches ⇒ count scan runs, component > 0). No querier-side pinned test was edited.Measured effect (local synthetic stores; components = count + materialize + registry)
Run #9 (post-merge dispatch) measures the isolated v8 delta vs run #8's honest baseline (expected saving ≈ the count-scan component: 0.61–1.56 MB per pair).
Regression tests
5 new (
rfc0031_single_pass.rs): complete-elision (pruning-count equality + saving == exactly the count scan's bytes), truncated fallback (rows = full total, pinned stats shape), exactly-at-limit fallback, empty-match elision, default-options two-pass shape intact.Invariants / hazards
No schema, miner, or ingest change. Hazard 6 (DSL vs engine surface): the opt-in is on the Rust API, nothing leaks into the DSL. B1/B2 gates unaffected (default path byte-identical).
Checks run
cargo fmt --all --check,cargo clippy --all-targets --all-features -- -D warnings(workspace-wide),cargo nextest run -p ourios-querier -p ourios-bench(296 passed, 14 skipped = pre-existing#[ignore]d container tests).🤖 Generated with Claude Code
https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y