Repository navigation
Range query key expansion uses one snapshot instead of per-step keys #583
Description
Activity
milindsrivastava1997 commented
on Aug 24, 2026 ContributorAuthorMore actionsConfirmed both bugs described above by reading the code:
Bug 1 — stale keys snapshot reused across all range-query output steps (
asap-query-engine/src/engines/simple_engine/mod.rs,execute_range_query_pipeline):merged_keysis fetched/merged once fromcontext.base.store_plan.keys_querybefore the per-timestamp loop (thecurrent_timeloop over[start_ms, end_ms]), and the sameexpansion_keysper group are reused for every output point. If the key set changes mid-range, early/late steps get phantom or missing series relative to what the key set actually was at that timestamp.Bug 1b — same defect in the binary-expr range path:
handle_binary_expr_range_promql→build_arm_range_context→finish_range_contextbuilds each arm'sRangeQueryExecutionContextthe same way, then feeds it to the sameexecute_range_query_pipeline. So each arm independently has the identical single-snapshot bug — confirmed via a RED test (range_query_binary_expr_arm_key_appearing_midrange_has_no_phantom_earlier_sample), not just "a likely candidate."Bug 2 —
keys_querywindow is instant-anchored, not range-aware (create_keys_query_params): it derives the keys window purely fromend_timestamp([end-window, end]forSetAggregator,[0, end]forDeltaSetAggregator).finish_range_contextwidens onlyvalues_queryto[start-lookback, end]and cloneskeys_queryunchanged — so even a single keys fetch is scoped to the wrong window:SetAggregatormisses labels that had real values earlier in the range but dropped out of the final instant's key snapshot;DeltaSetAggregator's[0,end]happens to be correct/necessary given it's a full-replay-from-start aggregator.Both bugs share one root cause and one fix: fetch/merge keys per output step, scoped to that step's own window (not once at
end), in bothexecute_range_query_pipelineand (transitively) the binary-expr per-arm path.RED tests added covering all three failure modes (
asap-query-engine/src/tests/native_range_query_tests.rs):range_query_dual_population_key_appearing_midrange_has_no_phantom_earlier_sample(DeltaSetAggregator phantom-add)range_query_set_aggregator_earlier_key_not_silently_dropped(SetAggregator earlier-key dropped)range_query_binary_expr_arm_key_appearing_midrange_has_no_phantom_earlier_sample(same phantom-add bug through the binary-expr arm path)
- added 5 commits that reference this issue
on Aug 24, 2026 - added a commit that references this issue
on Sep 3, 2026
execute_range_query_pipeline's dual-population handling (added in #582, fixing #580) fetches and mergeskeys_queryonce, anchored at the range'send, and reuses that single snapshot for every output timestamp in the range. If the key set changes partway through the queried interval — a label combination first appears, or stops appearing — every step still uses the final snapshot: keys get phantom samples before they existed, or keep appearing after they stopped, instead of reflecting the key set as it actually was at each step's time.Please also check other parts of the codebase for the same class of bug: any code path that fetches/merges a value or key set once and then reuses it across multiple output points in a time series, where the underlying set can legitimately change over that span.
handle_binary_expr_range_promql's per-arm range context (build_arm_range_context, which calls the samefinish_range_context/keys-snapshot logic per arm) is one likely candidate worth checking specifically.Related: keys_query window scoping is instant-anchored, not range-aware
finish_range_contextwidensvalues_queryto[start-lookback, end]for a range query, but cloneskeys_queryunchanged — its window is still computed from a single instant (create_keys_query_params, using onlyend_timestamp), which is wrong for both key aggregation types, in opposite directions:SetAggregator("latest window only"): keys window is[end-window_size, end]. Since the range loop iteratesmerged_keys(notall_data), a label with real values earlier in the range but absent from that final window's key snapshot never gets iterated at all.DeltaSetAggregator("all keys since start"): keys window is[0, end]. This is actually necessary because this aggregator is a "delta" version ofSetAggregator. It requires replaying every delta bucket back to the initial full-snapshot windowFixing this properly likely means fetching/merging keys per output step, scoped consistently with each step's own window, rather than once at the range's
end— which would fix both this scoping mismatch and the snapshot-staleness problem above as the same fix.