test(scheduler): freeze the plan each TPC-H query ends on, under both planners - #2343
Draft
Dandandan wants to merge 1 commit into
Draft
test(scheduler): freeze the plan each TPC-H query ends on, under both planners#2343Dandandan wants to merge 1 commit into
Dandandan wants to merge 1 commit into
Conversation
Dandandan
force-pushed
the
test/plan-snapshots
branch
from
August 18, 2026 07:33
c70da76 to
a903256
Compare
… planners The plan-stability suite pins each TPC-H query's staged plan under the static planner. AQE is the default planner since apache#2315, so the planner that actually runs these queries had nothing holding its plans in place — and plan regressions are quiet. All three defects found while comparing Ballista's TPC-H plans against Spark (apache#2338, apache#2342) returned correct rows with the suite green; one ran a whole stage in a single task. Plan each query through `AdaptivePlanner` as well and freeze the result beside the existing golden: approved/qN.txt DefaultDistributedPlanner, staged approved/qN.adaptive.txt AdaptivePlanner, the plan the job ends on The adaptive golden is the resolved plan, not the initial one: AQE re-plans after every stage completion, so the plan worth pinning is the last one, where `DynamicJoinSelectionExec` has become a concrete join and coalesce decisions are settled. None of the 22 goldens contains an unresolved node. `planner::adaptive_final_plan` drives that loop without executing anything, reporting each stage's own estimated output as the stage's result. A cluster measures real statistics and can resolve differently; what the goldens hold is that a code change did not move the plan. The goldens are sensitive to the defects that motivated them: applying the broadcast guard from apache#2342 fails q7, q8 and q9 — and only those three. Queries and statistics are the suite's own — SQL read from `benchmarks/queries/`, SF100 cardinalities over dataless providers — so this adds a second lowering of the same 22 queries, not a second fixture. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Dandandan
force-pushed
the
test/plan-snapshots
branch
from
August 18, 2026 07:47
a903256 to
825986c
Compare
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
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.
Which issue does this PR close?
None filed. Follow-up to the plan defects found while comparing Ballista's TPC-H plans against Spark with AQE (#2338, #2342).
Rationale for this change
tests/tpch_plan_stabilityalready freezes each TPC-H query's staged plan — but only under the static planner. Since #2315 AQE is the default, so the planner that actually runs these queries had nothing holding its plans in place.That matters because plan regressions are quiet. All three defects behind #2338 and #2342 returned correct rows with this suite green; one ran an entire stage in a single task.
What changes are included in this PR?
Each query is planned through
AdaptivePlanneras well, and the result frozen beside the existing golden:approved/qN.txtDefaultDistributedPlannerapproved/qN.adaptive.txtAdaptivePlannerThe adaptive golden is the resolved plan, not the initial one. AQE re-plans after every stage completion, so the plan worth pinning is the last one — where
DynamicJoinSelectionExechas become a concreteHashJoinExec/SortMergeJoinExec, exchanges carry their stage ids, and coalesce decisions are settled. None of the 22 goldens contains an unresolved node.planner::adaptive_final_plandrives that replan loop without executing anything: it resolves stages in dependency order, reporting each stage's own estimated output as that stage's result. A cluster measures real statistics and can therefore resolve differently — what these goldens hold is that a code change did not move the plan.The goldens are sensitive to the defects that motivated them. Applying the broadcast guard from #2342 on top of this branch fails
q7,q8andq9— and only those three. q7's golden currently records the pre-fix shape, withbroadcast=trueon the probe side of aCollectLeftjoin whose build side is the 25-rownationscan:Queries and statistics are the suite's own — SQL read from the canonical
benchmarks/queries/, SF100 cardinalities over dataless providers — so this is a second lowering of the same 22 queries, not a second fixture.dev/update-tpch-plan-stability.shregenerates both sets and the RAT glob (approved/*) already covers the new files.Supporting changes:
planner::adaptive_final_planis a new public entry alongside the already-publicDefaultDistributedPlanner;state::aqebecomespub(crate);AdaptivePlannergains an owned-plan accessor and itsfinalise_stage_internalwidens frompub(super)topub(crate). The two query loaders now share one path.Are these changes tested?
The 22 new goldens are the change.
cargo test -p ballista-schedulerpasses (362 + 25) and clippy--all-targetsis clean. The 22 existing static goldens are byte-identical after the loader refactor, verified by regenerating and diffing.Are there any user-facing changes?
One public addition to the scheduler crate,
planner::adaptive_final_plan, for callers that want AQE's resolved plan without running a job. No behavior change.