jayzhan211 opened a new pull request, #24574:
URL: https://github.com/apache/datafusion/pull/24574
## Rationale for this change
`ExtractLeafExpressions` rewrites `MoveTowardsLeafNodes` expressions into
projections aliased `__datafusion_extracted_N`. That prefix is reserved for
the
optimizer, but a user query can still contain it, so before rewriting, the
rule
scans the plan for existing `__datafusion_extracted_N` aliases and advances
the
shared `AliasGenerator` past them to avoid handing out a name that is already
taken.
That scan used `LogicalPlan::apply`, which does **not** descend into subquery
plans nested inside expressions — while the rewrite itself uses
`transform_down_with_subqueries`, which does. So a user alias living inside a
subquery was invisible to the guard, and extraction could generate that exact
same alias inside that same subquery. The extraction projection also passes
through all of its input's columns, so the duplicate name surfaces as an
ambiguous column reference (planning error) or, worse, silently binds to the
wrong column.
Concretely, given a subquery containing `... AS __datafusion_extracted_7`,
the
generator still handed out `__datafusion_extracted_1` and counted up from
there,
eventually colliding.
## What changes are included in this PR?
In `advance_generator_past_existing`:
- `plan.apply` → `plan.apply_with_subqueries`, so the guard covers the same
tree
the rewrite walks. This is the fix.
- `plan.expressions().iter().try_for_each(...)` →
`plan.apply_expressions(...)`.
`LogicalPlan::expressions()` deep-clones every expression of every node,
and
this scan runs over the whole plan on every invocation of the rule purely
to
*inspect* aliases. `apply_expressions` borrows instead. This also lets the
nested closure return its `Result<TreeNodeRecursion>` directly, dropping
the
`Ok::<(), DataFusionError>` turbofish.
No behavior change beyond the collision fix; no public API change.
## Are these changes tested?
Yes — new unit test `test_advance_generator_past_alias_in_subquery`. It
builds a
plan whose `IN (SELECT ...)` subquery contains a user-written
`AS __datafusion_extracted_7` and asserts the next generated alias is
`__datafusion_extracted_8`.
The test was confirmed to fail on the pre-fix code
(`left: "__datafusion_extracted_1"`, `right: "__datafusion_extracted_8"`)
and to
pass after. No existing test covered `advance_generator_past_existing` at
all.
Full `cargo test -p datafusion-optimizer` passes (763 + 26 lib/integration
tests,
5 doctests), plus `cargo fmt --all` and
`cargo clippy --all-targets --all-features -- -D warnings`.
## Are there any user-facing changes?
No API changes. Queries that use the reserved `__datafusion_extracted_`
prefix
inside a subquery no longer risk an ambiguous-column error or an incorrect
column binding when leaf expression pushdown is enabled.
--
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]
