fix(control-plane): match a Planner fragment against its own subtree - #781
Merged
Merged
Conversation
residual_nodes locates the part of a query that still runs at query time by re-parsing every subtree of the original and comparing the typed IR with `==`. That comparison includes the leaf scan's schema, and the schema is the one thing in the tree that cannot survive the round trip. Planner documents a PromQL leaf schema as usage-derived: "the (ts, value) floor + the labels the query references", marked `closed: false` precisely because it does not enumerate the row. A fragment resolved inside the whole query therefore carries every label the query mentions, while the same fragment re-parsed alone carries only the labels it mentions. So `sum by (label_0) (rate(data[1m]))` produces a residual whose leaf scan has columns [ts, value, label_0], while re-parsing the subtree `rate(data[1m])` produces [ts, value]. Same source, predicates, range, measures, reduction; one context-derived column apart. The match failed and the query was rejected with "Planner residual does not match any original query subtree", even though the fragment is that subtree. Compare open leaf schemas by containment instead. Containment is a prefix, not an arbitrary subset, because ColumnIds are positional: the floor comes first and context only appends, so a prefix keeps every column id in predicates and grouping keys meaning the same column on both sides. Closed catalog-backed SQL schemas do enumerate the row, so they keep exact equality, and every other part of the tree is still compared exactly. Nothing that identifies the computation is relaxed. The existing false-match guards discriminate on matchers, range and metric name, and all still pass; the new test checks a different metric, range, function and matcher are each still rejected under a widened leaf schema. Found while auditing bind failures in the issue-754 workload, where this rejected three single-query candidates and, in batch planning, fifteen across a ten-query workload. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
Sep 28, 2026
An earlier revision claimed every recorded defect was a regression introduced inside the #737 -> #728 stack, on the grounds that main is green on these paths. Diffing residual_nodes against main does not support that: the function is substantively identical there, and the stack changed only how the accuracy target is derived and the wording of the error message. main is green because its tests never plan these queries, not because the code is correct. That is the more worrying reading, so it should be the recorded one. The fragment mismatch is fixed on main by #781: a context-derived leaf schema was compared for equality against an isolated re-parse that cannot reproduce it, and open schemas are now compared by containment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The new tests were inserted between an existing `#[test]` and the function it applied to, so `grouped_query_residual_matches_its_own_subtree` carried two attributes and `workload_horizon_residual_keeps_semantic_equality` silently stopped being a test. A local `cargo test --lib` run hid both, since neither lint is an error without `-D warnings`. Move the new tests above the attribute and give the original its own back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
Sep 28, 2026
#781 is on main: an open PromQL leaf schema is context-derived, so comparing it for equality against an isolated re-parse rejected fragments that were the subtree. With that fixed, every recorded fragment mismatch is gone: three single-query candidates, one in shared-rate and fifteen in all-ten. Removing them exposed a second schema incompatibility in all-ten that had been hidden behind the earlier rejection, so that count goes from 1 to 2. That is what pinning exact counts is for; a test that only checked "some non-empty reason" would have absorbed both the fix and the newly reachable defect without saying anything. Verified on this branch with the fix applied: level 1 is green on all four tests, and the two data_plane dashboard tests that failed with the same fragment-mismatch error, repeated_dashboard_executes_multiple_selected_panes and shared_exact_dashboard_executes_selected_workload, both pass. Co-Authored-By: Claude Opus 5 (1M context) <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.
residual_nodesmaps the query-time part of a plan back onto a subtree of the original query by re-parsing each subtree and comparing typed IR with==. The comparison includes the leaf scan schema, which is the one part of the tree that cannot survive that round trip.Planner documents a PromQL leaf schema as usage-derived, "the
(ts, value)floor + the labels the query references", and marks itclosed: falsebecause it deliberately does not enumerate the row. A fragment resolved inside the whole query carries every label the query mentions; the same fragment re-parsed alone carries only the labels it mentions.Concretely, for
sum by (label_0) (rate(data[1m])):Everything else is identical: reduction, measures, output_names, range, source, predicates, time_index. The subtree
rate(data[1m])was rejected as "not any original query subtree" although it is exactly that subtree.Change
Open leaf schemas are compared by containment. Containment is a prefix, not an arbitrary subset, because
ColumnIds are positional: the floor comes first and context only appends, so a prefix keeps every column id inpredicatesand grouping keys meaning the same column on both sides. Closed (catalog-backed SQL) schemas still require exact equality, and every other node is compared exactly.Why this is not a loosening
Nothing that identifies the computation is relaxed. The pre-existing false-match guards discriminate on matchers (
job="api"vs"worker"), range ([1m]vs[2m]) and metric name, and all pass unchanged. The new test asserts that under a widened leaf schema a different metric, a different range, a different function and a different matcher are each still rejected.Validation
cargo test -p control_plane --lib: 433 passed, 0 failed.cargo fmt --checkandcargo clippy -p control_plane --libclean.Provenance
Found while auditing bind failures in the issue-754 workload. It rejected three single-query candidates there, and fifteen in batch planning across a ten-query workload. The defect is on
main;mainis green only because its tests do not plan these queries.🤖 Generated with Claude Code