Skip to content

Remove the redundant join-flattening precheck in EliminateCrossJoin #25680

Description

@Toby1009

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:

  • 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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions