refactor(query-engine): fetch non-exact store queries via a window-grid walk of exact lookups - #616
Merged
Merged
Conversation
…id walk of exact lookups Replaces execute_store_query's use of the tolerant Store::query_precomputed_output scan with scan_windows_via_exact, which walks the aggregation's window grid (bucket_step_ms apart) and probes each position with Store::query_precomputed_output_exact instead. Falls back to the old scan for DeltaSetAggregator queries, whose [0, end_timestamp] "all keys ever seen" range would otherwise make the grid-walk pathologically slow. Exposed and fixed several pre-existing test-fixture bugs along the way (zero-width buckets, Tumbling configs with slide != window_size, keys buckets narrower than window_size_ms) where fixtures didn't match the bucket shapes worker.rs actually produces. Also surfaced a real, pre-existing bug in range queries over Sliding-window aggregations (tracked separately as #608) -- pinned with two new RED tests, both #[ignore]'d pending that fix. Co-authored-by: Claude Sonnet 5 <[email protected]>
milindsrivastava1997
changed the title
…dow-grid # Conflicts: # asap-query-engine/src/tests/datafusion/plan_execution_temporal_tests.rs
milindsrivastava1997
marked this pull request as ready for review
August 26, 2026 01:31
This was referenced Aug 26, 2026
milindsrivastava1997
added a commit
that referenced
this pull request
Aug 26, 2026
StoreQueryParams::is_exact_query was a bool computed once and threaded through StoreQueryPlan, then consulted by execute_store_query to pick between store fetch strategies. Every StoreQueryParams builder had to independently derive it correctly, and past call sites already drifted out of sync more than once (#580, #582, #608). Since #616 replaced the tolerant-scan branch with scan_windows_via_exact (a grid-walk of exact lookups), the flag's only remaining job was choosing between that grid-walk and a single direct exact call -- but a range exactly one window wide already makes scan_windows_via_exact degenerate to a single exact lookup. So it can go away entirely: create_store_query_plan still narrows the Sliding-instant values query to one window's width (unchanged), and execute_store_query now unconditionally calls scan_windows_via_exact. The one other thing is_exact_query did -- telling execute_and_merge_store_queries whether to use Sliding or Tumbling merge semantics -- is unrelated to fetch mechanism and is now passed explicitly as a WindowType parameter, sourced from create_store_query_plan's (now three-element) return value and threaded onto QueryExecutionContext. Also adds window_semantics_consistency_tests.rs: hardening tests written against the observable PromQL query surface (not internal struct/function names), covering Sliding/Tumbling instant-vs-range agreement, keys queries over each WindowType, and window-grid boundary cases. Fixes #613. Co-authored-by: Claude Sonnet 5 <[email protected]>
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.
What
execute_store_query's non-exact branch used to call the store's tolerant scan (query_precomputed_output). It now walks the aggregation's window grid and callsquery_precomputed_output_exactat each position instead (scan_windows_via_exact).DeltaSetAggregatorqueries (which span[0, end_timestamp]— "all keys ever seen") fall back to the old scan, since walking that range position-by-position would be far too slow.Why
Direct request to replace the tolerant scan with exact lookups for consistency/perf. Along the way, this exposed several test fixtures that didn't match what the real ingestion pipeline (
worker.rs) actually stores — zero-width buckets, Tumbling configs whereslide_interval_ms != window_size_ms, and keys buckets narrower thanwindow_size_ms. Those are fixed here too.Known gap (not introduced by this PR)
Range queries over Sliding-window aggregations already have a pre-existing bug, tracked separately as #608 — this work surfaced it (previous fixtures never modeled realistic Sliding data) but didn't cause it. Two new tests pin this and are
#[ignore]d pending that fix; see the comment on #608 for details.Testing
Full test suite passes (588 passed, 7 ignored — 2 for #608, 5 pre-existing). New adversarial test file (
exact_window_grid_adversarial_tests.rs) covers boundary/containment edge cases, overlapping Sliding windows, sealed/mutable epoch spans, and theDeltaSetAggregatorwide-range perf case directly against theStoretrait contract.