Skip to content

test(scheduler): freeze the plan each TPC-H query ends on, under both planners - #2343

Draft
Dandandan wants to merge 1 commit into
apache:mainfrom
Dandandan:test/plan-snapshots
Draft

test(scheduler): freeze the plan each TPC-H query ends on, under both planners#2343
Dandandan wants to merge 1 commit into
apache:mainfrom
Dandandan:test/plan-snapshots

Conversation

@Dandandan

@Dandandan Dandandan commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

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_stability already 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 AdaptivePlanner as well, and the result frozen beside the existing golden:

file planner contents
approved/qN.txt DefaultDistributedPlanner staged plan, stage by stage (unchanged)
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 HashJoinExec/SortMergeJoinExec, exchanges carry their stage ids, and coalesce decisions are settled. None of the 22 goldens contains an unresolved node.

planner::adaptive_final_plan drives 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, q8 and q9 — and only those three. q7's golden currently records the pre-fix shape, with broadcast=true on the probe side of a CollectLeft join whose build side is the 25-row nation scan:

HashJoinExec: mode=CollectLeft, on=[(n_nationkey@0, c_nationkey@3)], filter=...
  FilterExec: n_name@1 = GERMANY OR n_name@1 = FRANCE
    StatsExec: rows=25
  ExchangeExec: partitioning=None, plan_id=11, stage_id=5, stage_resolved=true, broadcast=true

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.sh regenerates both sets and the RAT glob (approved/*) already covers the new files.

Supporting changes: planner::adaptive_final_plan is a new public entry alongside the already-public DefaultDistributedPlanner; state::aqe becomes pub(crate); AdaptivePlanner gains an owned-plan accessor and its finalise_stage_internal widens from pub(super) to pub(crate). The two query loaders now share one path.

Are these changes tested?

The 22 new goldens are the change. cargo test -p ballista-scheduler passes (362 + 25) and clippy --all-targets is 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.

@Dandandan Dandandan changed the title test(scheduler): pin the expected static and distributed plans test(scheduler): freeze the adaptive plan for every TPC-H query too Aug 18, 2026
@Dandandan
Dandandan force-pushed the test/plan-snapshots branch from c70da76 to a903256 Compare August 18, 2026 07:33
… 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 Dandandan changed the title test(scheduler): freeze the adaptive plan for every TPC-H query too test(scheduler): freeze the plan each TPC-H query ends on, under both planners Aug 18, 2026
@Dandandan
Dandandan force-pushed the test/plan-snapshots branch from a903256 to 825986c Compare August 18, 2026 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant