Toby1009 opened a new issue, #25680: URL: https://github.com/apache/datafusion/issues/25680
### Is your feature request related to a problem or challenge? `EliminateCrossJoin` calls `can_flatten_join_inputs` before `flatten_join_inputs`, walking the inner-join subtree twice even though the precheck can no longer reject either call site. In the [current implementation](https://github.com/apache/datafusion/blob/95bb0a0dfa48ca20d017faa55b2fc96ee2f300fe/datafusion/optimizer/src/eliminate_cross_join.rs#L311): - Both callers already establish that the root is an `Inner` join: the filter branch checks its input, and the other branch matches the join type directly. - The helper rejects non-inner roots, but only recurses into children that are themselves inner joins. It has no other rejection condition, so it always returns `true` for these callers. The check originally rejected inner joins with filters to avoid losing their predicates. #13025 taught `flatten_join_inputs` to collect and preserve those filters and removed that rejection condition, but kept the recursive precheck and the comment requiring both helpers to stay in sync. ### Describe the solution you'd like Keep the cleanup local to `datafusion/optimizer/src/eliminate_cross_join.rs`: - Remove `can_flatten_join_inputs` and its two redundant call-site guards. - Update `flatten_join_inputs` documentation to describe collecting inner-join inputs, keys, and filters, with other nodes retained as inputs. - Make the private `flatten_join_inputs` helper infallible: it currently returns `Result<()>` but only propagates recursive calls and returns `Ok(())`. This should preserve the same optimized plans while removing a preliminary traversal and the need to keep two traversal implementations aligned. No SQL behavior or public API change is intended. Planning performance has not been measured. ### Describe alternatives you've considered Keep a root-only precheck. This would still duplicate the existing call-site guards, so removing it seems simpler. ### Additional context Validation should cover both entry paths (a filter over an inner join and a bare inner join), nested joins with filters, non-inner join boundaries, and preservation of output schemas and null-equality settings. Existing `eliminate_cross_join` tests cover several of these cases; add focused regression coverage where needed and compare planning performance for multi-join queries. Keep the separate `plan_has_joins` fast path and child/subquery rewriting unchanged. #23686 touches the same file but addresses schema refresh in `rewrite_children`, a separate concern. -- 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]
