LuciferYang opened a new pull request, #57790: URL: https://github.com/apache/spark/pull/57790
### What changes were proposed in this pull request? `canPlanAsBroadcastHashJoin` already computed an `Option[BuildSide]` internally and then discarded the direction with `.isDefined`. A caller that needs the direction therefore had to reimplement the same composition: `PushDownJoinThroughUnion` (SPARK-58449) did exactly that, so one planner decision was modelled twice, with each model incomplete and the coupling invisible in the code. This PR lifts the composition into `JoinSelectionHelper.getBroadcastHashJoinBuildSide`, which returns `Option[BuildSide]`, and redefines `canPlanAsBroadcastHashJoin` as `.isDefined` on it. The truth table of `canPlanAsBroadcastHashJoin` is unchanged, including its two deliberate over-approximations of `JoinSelection`: join keys no hash join supports still reach the size branch, and the method does not model `SHUFFLE_MERGE` or `SHUFFLE_REPLICATE_NL` hints. Both are now stated in the scaladoc instead of being implicit. `PushDownJoinThroughUnion` then drops its own `canPlanAsBroadcastHashJoin` conjunct, since `None` from the new method already covers the equi-key and hash-joinable-key requirements, and shares a `branchJoin` helper between the rewrite and the guard's probe plan. The probe now rewrites the join condition to the branch output, which the previous guard did not need to do because it never extracted equi-join keys. Making the helper fully faithful to `JoinSelection` would mean returning `None` under a `SHUFFLE_MERGE` or `SHUFFLE_REPLICATE_NL` hint. That flips `canPlanAsBroadcastHashJoin` from true to false for a hinted join whose size still qualifies, which changes what `PushDownLeftSemiAntiJoin` pushes down, so it is a behavior change rather than a refactor and is left for a separate ticket. ### Why are the changes needed? Two partial models of the same planner decision drift apart. Strengthening one of them, for example teaching `canPlanAsBroadcastHashJoin` about a hint it currently ignores, silently leaves the other stale, and nothing in the code says the two have to agree. ### Does this PR introduce _any_ user-facing change? No. `canPlanAsBroadcastHashJoin` keeps its truth table, and `spark.sql.optimizer.pushDownJoinThroughUnion.enabled` remains false by default. ### How was this patch tested? `JoinSuite.assertJoin` now also asserts that `getBroadcastHashJoinBuildSide` agrees with the build side the planner picked, which covers every case in that suite that reaches a broadcast hash join. `JoinSelectionHelperSuite` gains five cases asserting the direction itself: a hint on either or both sides, the fall back to the smaller side, a shuffle hash hint, no equi-join keys, and a null-aware anti join. The direction had no coverage before this PR, so both were checked by mutation: inverting `getSmallerSide` fails four cases, and returning `BuildLeft` for the null-aware anti join fails exactly the new case for it. Existing suites pass: `JoinSelectionHelperSuite` (19), `PushDownJoinThroughUnionSuite` in catalyst (24) and core (9), and `JoinSuite` (58). ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
