You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
finish_range_context (asap-query-engine/src/engines/simple_engine/promql.rs:615) unconditionally sets extended_store_plan.values_query.is_exact_query = false, regardless of the aggregation's actual WindowType. This is the bug named in #580 (closed by #582) and re-flagged as still-live in a comment on #581 — #582's changes never touched this line.
Effect: a PromQL range query over a Sliding-window aggregation is routed to the overlap-scan fetch (query_precomputed_output) instead of the exact-window fetch (query_precomputed_output_exact), then merged downstream as if it were Tumbling (window_type = if is_exact_query { Sliding } else { Tumbling }, mod.rs:558) — wrong values, silently, no error. Instant queries don't have this bug: create_store_query_plan correctly derives is_exact_query = window_type == WindowType::Sliding (mod.rs:427).
Fix
Route Sliding-window range queries through query_precomputed_output_exact, once per output step, deriving each step's exact window bounds from the value aggregation's AggregationConfig (window_size_ms) the same way the instant path already does for its single window. Tumbling stays on the scan call (query_precomputed_output) unchanged — it's already correct, and (per #607) the scan call is read-locked while exact-query is currently write-locked, so there's no reason to move Tumbling off it.
Concretely, this likely means:
execute_range_query_pipeline's per-step loop (mod.rs:1541+) gains a Sliding-specific branch: instead of pulling from a single wide all_data fetch + bucket_map window-scan (today's Tumbling-shaped approach), each step computes its exact window and calls execute_store_query with an exact-mode StoreQueryParams for that step.
Naturally wants # (batched exact-query API) to avoid N sequential store round-trips per range query, but is not blocked on it — N sequential exact calls is a valid, correct first cut.
On is_exact_query itself
A comment on #581 proposed removing is_exact_query as a threaded bool entirely once this is fixed, to eliminate "one more place the two paths can independently drift." Worth revisiting the field's shape as part of this fix (e.g., mirror the derivation in one shared helper called by both create_store_query_plan and finish_range_context), but note the keys-side query (create_keys_query_params) deliberately hardcodes exact=false regardless of the key aggregation's WindowType ("keys always use range queries") — any refactor of this field must preserve that as an explicit, named choice, not something a blanket "derive from WindowType" rule would get right by accident.
Testing
Per project convention: TDD. Write a RED test first that pins today's actual bug (a range query over a Sliding-window aggregation returns wrong values / wrong window composition), then fix.
Related: #581 (broader instant/range fetch-merge unification — this is one of the "must fix before/during step 4" items called out there), #580 (originally named this, incompletely fixed by #582), #607 (removes the write-lock cost this fix would otherwise multiply).
finish_range_context(asap-query-engine/src/engines/simple_engine/promql.rs:615) unconditionally setsextended_store_plan.values_query.is_exact_query = false, regardless of the aggregation's actualWindowType. This is the bug named in #580 (closed by #582) and re-flagged as still-live in a comment on #581 — #582's changes never touched this line.Effect: a PromQL range query over a Sliding-window aggregation is routed to the overlap-scan fetch (
query_precomputed_output) instead of the exact-window fetch (query_precomputed_output_exact), then merged downstream as if it were Tumbling (window_type = if is_exact_query { Sliding } else { Tumbling },mod.rs:558) — wrong values, silently, no error. Instant queries don't have this bug:create_store_query_plancorrectly derivesis_exact_query = window_type == WindowType::Sliding(mod.rs:427).Fix
Route Sliding-window range queries through
query_precomputed_output_exact, once per output step, deriving each step's exact window bounds from the value aggregation'sAggregationConfig(window_size_ms) the same way the instant path already does for its single window. Tumbling stays on the scan call (query_precomputed_output) unchanged — it's already correct, and (per #607) the scan call is read-locked while exact-query is currently write-locked, so there's no reason to move Tumbling off it.Concretely, this likely means:
execute_range_query_pipeline's per-step loop (mod.rs:1541+) gains a Sliding-specific branch: instead of pulling from a single wideall_datafetch +bucket_mapwindow-scan (today's Tumbling-shaped approach), each step computes its exact window and callsexecute_store_querywith an exact-modeStoreQueryParamsfor that step.On
is_exact_queryitselfA comment on #581 proposed removing
is_exact_queryas a threaded bool entirely once this is fixed, to eliminate "one more place the two paths can independently drift." Worth revisiting the field's shape as part of this fix (e.g., mirror the derivation in one shared helper called by bothcreate_store_query_planandfinish_range_context), but note the keys-side query (create_keys_query_params) deliberately hardcodes exact=false regardless of the key aggregation'sWindowType("keys always use range queries") — any refactor of this field must preserve that as an explicit, named choice, not something a blanket "derive from WindowType" rule would get right by accident.Testing
Per project convention: TDD. Write a RED test first that pins today's actual bug (a range query over a Sliding-window aggregation returns wrong values / wrong window composition), then fix.
Related: #581 (broader instant/range fetch-merge unification — this is one of the "must fix before/during step 4" items called out there), #580 (originally named this, incompletely fixed by #582), #607 (removes the write-lock cost this fix would otherwise multiply).