fix: scan subqueries when advancing the extracted-alias generator - #24574
Open
jayzhan211 wants to merge 2 commits into
Open
fix: scan subqueries when advancing the extracted-alias generator#24574jayzhan211 wants to merge 2 commits into
jayzhan211 wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #24574 +/- ##
========================================
Coverage 81.37% 81.38%
========================================
Files 1116 1116
Lines 397509 397973 +464
Branches 397509 397973 +464
========================================
+ Hits 323461 323878 +417
- Misses 55110 55129 +19
- Partials 18938 18966 +28 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
adriangb
approved these changes
Aug 22, 2026
adriangb
left a comment
Contributor
There was a problem hiding this comment.
Is there a SQL / SLT reproducer?
Contributor
Author
|
I add slt test! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
ExtractLeafExpressionsrewritesMoveTowardsLeafNodesexpressions intoprojections aliased
__datafusion_extracted_N. That prefix is reserved for theoptimizer, but a user query can still contain it, so before rewriting, the rule
scans the plan for existing
__datafusion_extracted_Naliases and advances theshared
AliasGeneratorpast them to avoid handing out a name that is alreadytaken.
That scan used
LogicalPlan::apply, which does not descend into subqueryplans nested inside expressions — while the rewrite itself uses
transform_down_with_subqueries, which does. So a user alias living inside asubquery 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, thegenerator still handed out
__datafusion_extracted_1and 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 treethe rewrite walks. This is the fix.
plan.expressions().iter().try_for_each(...)→plan.apply_expressions(...).LogicalPlan::expressions()deep-clones every expression of every node, andthis scan runs over the whole plan on every invocation of the rule purely to
inspect aliases.
apply_expressionsborrows instead. This also lets thenested closure return its
Result<TreeNodeRecursion>directly, dropping theOk::<(), 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 aplan whose
IN (SELECT ...)subquery contains a user-writtenAS __datafusion_extracted_7and 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 topass after. No existing test covered
advance_generator_past_existingat all.Full
cargo test -p datafusion-optimizerpasses (763 + 26 lib/integration tests,5 doctests), plus
cargo fmt --allandcargo clippy --all-targets --all-features -- -D warnings.Are there any user-facing changes?
No API changes. Queries that use the reserved
__datafusion_extracted_prefixinside a subquery no longer risk an ambiguous-column error or an incorrect
column binding when leaf expression pushdown is enabled.