jayzhan211 opened a new pull request, #27224: URL: https://github.com/apache/datafusion/pull/27224
## Which issue does this PR close? - Part of #26481. ## Rationale for this change A hash join that reaches `EnsureRequirements` in `PartitionMode::Auto` gets no repartitioning, because `Auto` declares no distribution requirement. When `JoinSelection` runs after enforcement and picks `Partitioned`, the plan then fails the final check. Running the sqllogictest suite with `JoinSelection` moved after `EnsureRequirements` (with a local harness, not part of this PR), 772 plans failed the final check at 4 partitions on main at 9d15cf079f; with this change, 224 do. This PR makes `EnsureRequirements` pick the mode of any hash join it still finds in `Auto`, so the requirements it enforces match the join. In the default rule order `JoinSelection` runs first and decides the mode, so no plan changes there. ## What changes are included in this PR? - `resolve_auto_join_mode` in `ensure_requirements/enforce_distribution.rs` (start reading here): `Partitioned` when the join may repartition its inputs (`target_partitions > 1`, `repartition_joins`, not null-aware), `CollectLeft` otherwise. `EnsureRequirements` calls it in its first top-down pass, next to the interleave normalization. - The decision I'd most like reviewed: this choice uses no statistics. Both modes are valid, but `Partitioned` may shuffle a small build side that `JoinSelection` would have collected. - Test hook: `moved_rule` in `sqllogictest/src/test_context.rs` runs a named test file with one default rule moved, like the existing custom rule list for `sort_pushdown.slt`. ## What is the testing strategy for this PR? - `rule_order_join_selection.slt` runs `JoinSelection` after `EnsureRequirements`. Its inner and left join queries fail on main with `SanityCheckPlan: order and distribution requirements stopped holding after physical optimizer rule 'join_selection'`, and pass with this change. - The full sqllogictest suite (including TPC-H), the physical optimizer integration tests and the crate's unit tests pass unchanged. ## Are there any user-facing changes? No API change, and no plan change in the default rule order. Downstream impact: - Rule lists that run `EnsureRequirements` before `JoinSelection` now get a valid plan for a hash join still in `Auto`, instead of a planning error. - Not addressed here: in the measurement above, 224 plans still fail. Those are rewrites `JoinSelection` makes after enforcement, for example swapping nested-loop and cross joins, or changing a hash join's mode after enforcement decided it. -- 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]
