fix(query-engine): sliding windows validate range-query step against slide interval - #562
Open
milindsrivastava1997 wants to merge 3 commits into
Open
fix(query-engine): sliding windows validate range-query step against slide interval#562milindsrivastava1997 wants to merge 3 commits into
milindsrivastava1997 wants to merge 3 commits into
Conversation
Grilling-session writeup: unify instant and range-query fetch onto the existing range-query pipeline's stride-W walk instead of adding a separate exact-lookup loop, corrected read-count-threshold formula for the resulting fetch-vs-merge distinction, and a TDD staging plan. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Keep Tumbling's validate_range_query_params check untouched instead of deleting it; add a real (not vestigial) Sliding-only step % slide_interval_ms check that guarantees grid alignment across range-query steps by construction, replacing per-step floor-align with a one-time start alignment for the range case. - Write up the PR A-D stacked-PR sequencing discussed in review: execution (B/C) built and tested against hand-registered configs before capability matching (D) is relaxed to let real queries reach it, with A independent and, like the rest of the stack, inert in production until #555 unblocks the planner. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
validate_range_query_params rejected any range-query step that wasn't a multiple of window_size_ms, regardless of window type. For Sliding this is stricter than necessary: the per-step walk only needs step to be a multiple of slide_interval_ms to stay grid-aligned across steps (S | W always holds for a valid sliding config), so step == slide_interval_ms (the natural "refresh every slide interval" cadence) was being wrongly rejected. Tumbling's existing check is untouched. PR A of the #557 stack (see .design_docs/sliding-window-query-engine-design.md). Inert in production: should_use_sliding_window() is still hardcoded false, so no live query can reach the Sliding branch yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Part of #557. First of a 4-PR stack (design doc:
.design_docs/sliding-window-query-engine-design.md).What
validate_range_query_paramsrejected any range-querystepthat wasn't a multiple ofwindow_size_ms, regardless of window type. For Sliding this is stricter than necessary — see the doc's §5 for the full reasoning. Adds a Sliding-only branch checkingstep % slide_interval_ms == 0instead; Tumbling's existing check is untouched.Why this is safe to merge on its own
should_use_sliding_window()is still hardcodedfalse— the planner cannot emit aSlidingconfig today, so nothing in production can reach the new branch. Tumbling's behavior is byte-for-byte unchanged.Stack
This is PR A. PR B (execution unification), PR C (
cleanup.rs), and PR D (capability_matching.rsrelaxation — the one that actually activates multi-window Sliding serving) will stack on top of this one, each as its own reviewable PR.Testing
7 new unit tests (
validate_range_query_params_tests) covering both window types plus the pre-existingstart < end/step > 0checks. Full workspacecargo test,cargo clippy -D warnings,cargo fmt --checkall pass.