wangyum commented on PR #59636: URL: https://github.com/apache/spark/pull/59636#issuecomment-6419527364
Thanks for the thorough review. Pushed 081b0274e1b addressing the items below. Fixed: - **#1** (a bucketed join nested inside a broadcast build side, no `BuildLeft` needed): confirmed by reproducing the stamping bug against the pre-fix code, then added `SPARK-60382: a bucketed join inside a broadcast build side keeps its own bucket count` covering the `BuildRight`-nested shape you sketched. Also widened the PR description's scope beyond `BuildLeft`. - **#2** (AQE-off-only coverage): reproduced the 0-row result with AQE enabled against the pre-fix code. The end-to-end test now loops `spark.sql.adaptive.enabled` over `true`/`false` using `AdaptiveSparkPlanHelper.collect`. - **#4** (nested-loop divisible rewrite untested): added the `BroadcastNestedLoopJoinExec` variant alongside the existing broadcast-hash-join one. - **#5** (assert `BuildLeft` was actually chosen): added. - **#6** (no-op confs / `ConvertToLocalRelation` exclusion overwrite): confirmed both claims (join hints bypass `ReorderJoin`; `withSQLConf` overwrites rather than appends the suite-wide exclusion) and dropped `AUTO_BROADCASTJOIN_THRESHOLD`, `CBO_ENABLED`, and the `ReorderJoin` exclusion from the test. - **#7** (ratio-check behavior change undocumented): added to the "Does this PR introduce any user-facing change?" section. - **#8** (`findBucketedScan` doc overstated its guarantee): reworded to point at `satisfiesOutputPartitioning` as the actual guard. Left as-is: - **#3** (branch-3.5 compile/semantics): out of scope for this master-only PR; will handle it if and when this gets backported. - **#9** (missing `Co-Authored-By` trailer in the generative-AI disclosure section): still deciding how to handle, will follow up separately. - **#10-14** (style nits in the production rule file and test helper duplication): left alone for now to avoid re-review churn on already-correct code; happy to take a pass if you'd still like a tidier diff. -- 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]
