fix: [branch-54] disable the SortMergeJoinExec broadcast conversion by default - #2130
Merged
andygrove merged 1 commit intoJul 22, 2026
Merged
Conversation
The static distributed planner converts a `SortMergeJoinExec` with a small build side into a broadcast `CollectLeft` hash join, gated by `ballista.optimizer.broadcast_sort_merge_join_enabled`. That conversion produces incorrect results, and the key defaults to `true`, so the bad path is on out of the box. It fires readily: Ballista sets `datafusion.optimizer.prefer_hash_join = false` (see apache#1648), so joins are planned as sort-merge unless the user opts out, and the conversion then applies to any join with a side under `broadcast_join_threshold_bytes` (default 10 MB). Flip the default to `false`. The key is kept so anyone depending on the old plan shape can opt back in, which keeps this change non-breaking for the 54 line. Removing the key outright is left to 55.0.0. The conversion also worked against the reason sort-merge is the default. DataFusion's hash join has no spill support, so a `CollectLeft` build side must fit in memory in every parallel task, while sort-merge spills. Regenerate the 11 affected TPC-H plan-stability goldens. They come out byte-identical to the plans produced on main by apache#2072, which removed the conversion entirely.
phillipleblanc
approved these changes
Jul 22, 2026
andygrove
marked this pull request as ready for review
July 22, 2026 02:57
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?
Backport of the correctness fix in #2072 to
branch-54. Partially addresses #2046.Rationale for this change
54.0.0added a static-planner rewrite that converts aSortMergeJoinExecwith a small build side into a broadcastCollectLefthash join, gated byballista.optimizer.broadcast_sort_merge_join_enabled. The rewrite produces incorrect results, and the key defaults totrue, so the bad path is on out of the box on54.0.0and54.1.0.This is not an edge-case path. Ballista sets
datafusion.optimizer.prefer_hash_join = falseby default (#1648), so joins are planned asSortMergeJoinExecunless a user opts out, and the conversion then fires on any join with a side underbroadcast_join_threshold_bytes(default 10 MB).The conversion also worked against the reason sort-merge is the default. DataFusion's hash join has no spill support, so a
CollectLeftbuild side must fit in memory in every parallel task, while sort-merge spills.On
main, #2072 removed the rewrite and its config key outright. Deleting a public config key is a breaking change, which is not appropriate for the 54 patch line, so this PR takes the non-breaking half of that fix: flip the default tofalseand keep the key so anyone depending on the old plan shape can opt back in. Removal stays onmainfor55.0.0.What changes are included in this PR?
ballista.optimizer.broadcast_sort_merge_join_enabledtofalse.broadcast_sort_merge_join_enabled_by_defaultunit test to pin the new default (renamed to..._disabled_by_default).54.0.0upgrade guide, with a warning for users already on54.0.0/54.1.0.The planner code and the
distributed_broadcast_sort_merge_join_plantest are untouched: that test opts in explicitly, so it still covers the conversion for anyone who enables it.Are there any user-facing changes?
Yes, and that is the point of the fix. Joins that were silently converted to broadcast hash joins now stay as sort-merge joins, so query plans change and incorrect results go away. No public API is removed or renamed, and no proto/wire format changes, so this is not a breaking change.
Verification: the regenerated goldens are byte-identical to the ones #2072 produced on
main, confirming this default flip yields exactly the same plans as the upstream removal.cargo fmt --check, clippy onballista-core/ballista-schedulerwith--all-targets --all-features, and the fullballista-core+ballista-schedulertest suites are clean.