kakiuwang-ui opened a new pull request, #25294:
URL: https://github.com/apache/datafusion/pull/25294
## Which issue does this PR close?
- Closes #15046.
## Rationale for this change
An extension node can name columns whose predicates must not be pushed past
it, via `UserDefinedLogicalNode::prevent_predicate_push_down_columns`. A
predicate whose only dependency on those columns runs through a correlated
subquery is pushed down anyway, and the resulting plan does not just perform
worse — it fails to execute.
Using the `NoopPlan` node already present in the optimizer's own tests,
which refuses push-down of column `c`:
```
Filter: EXISTS (SELECT sq.a FROM sq WHERE outer_ref(test.c) = sq.a)
NoopPlan
TableScan: test
```
optimizing that plan today gives
```
Optimizer rule 'push_down_filter' failed
Invalid (non-executable) plan after Optimizer rule: push_down_filter
In/Exist/SetComparison subquery can only be used in Projection, Filter,
TableScan,
Window functions, Aggregate and Join plan nodes, but was used in
[NoopPlan]
```
The predicate is moved below the node that asked to keep it, lands somewhere
subqueries are not permitted, and the invariant check turns that into a query
failure.
The cause is the one #15046 describes. The check is
```rust
!expr.column_refs().iter().any(|c| prevent_cols.contains(&c.name))
```
and `column_refs` collects only `Expr::Column`. A subquery records the outer
columns it correlates on in `Subquery::outer_ref_columns`, which `Expr`'s
traversal does not descend into, so `EXISTS (... WHERE outer.c = ...)` reports
no column references at all and looks unconditionally safe to push.
## What changes are included in this PR?
A `references_any_column` helper in `push_down_filter.rs` that asks
`column_refs` first and then, for `Expr::Exists`, `Expr::InSubquery` and
`Expr::ScalarSubquery`, also looks at the `outer_ref_columns` those carry. The
extension-node branch uses it instead of `column_refs` alone. Nothing else
changes.
**I deliberately did not change `column_refs` itself**, which is what the
issue title asks for. It has many callers whose behaviour would shift —
`optimize_projections`, CSE, window and join handling all use it, and several
of them want "columns produced by this plan's own schema", not outer references
belonging to a parent. Widening it looks like a separate change with its own
risk, so this PR fixes the consumer that has a demonstrable failure. Happy to
look at the broader change instead if you would rather that were the fix.
## What is the testing strategy for this PR?
New unit test
`push_down_filter::tests::user_defined_plan_outer_referenced_column`, next to
the existing `user_defined_plan` test and reusing its `NoopPlan`. It asserts
the `EXISTS` predicate stays above the node.
The test discriminates: with the outer-reference collection short-circuited
out, it fails with the invalid-plan error quoted above; with the fix it passes.
`datafusion-optimizer` (808) and `datafusion-expr` (262) pass, as does
`sqllogictest`.
## Are there any user-facing changes?
No public API changes. Plans change only where an extension node declares
`prevent_predicate_push_down_columns` and a predicate correlates a subquery on
one of those columns — previously an invalid plan, now a correct one. No
in-tree node declares those columns outside tests, so no bundled plan snapshots
move.
Two adjacent things found while working on this, neither touched here:
- `find_out_reference_exprs` does not see these outer references either, for
the same reason, so it cannot be used as the collection mechanism. That is the
same traversal gap as #16147, which @AdamGS has taken.
- The equivalent guard for correlated subqueries in other push-down branches
was not audited; this PR only covers the extension-node branch that #15046
reports.
--
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]
