-
Notifications
You must be signed in to change notification settings - Fork 29.2k
[SPARK-55794][FOLLOWUP][SQL] Wrap outer star expansion results in Alias in ResolveReferences #55606
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1868,7 +1868,14 @@ class Analyzer( | |||||||||
| // Only Project, Aggregate, CollectMetrics can host star expressions. | ||||||||||
| case u @ (_: Project | _: Aggregate | _: CollectMetrics) => | ||||||||||
| Try(s.expand(u.children.head, resolver)) match { | ||||||||||
| case Success(expanded) => expanded.map(wrapOuterReference) | ||||||||||
| case Success(expanded) => | ||||||||||
| expanded.map { | ||||||||||
| case alias: Alias => | ||||||||||
| alias.withNewChildren(Seq(wrapOuterReference(alias.child))) | ||||||||||
| .asInstanceOf[Alias] | ||||||||||
|
Comment on lines
+1874
to
+1875
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor:
Suggested change
|
||||||||||
| case e => | ||||||||||
| Alias(wrapOuterReference(e), toPrettySQL(e))() | ||||||||||
|
Comment on lines
+1876
to
+1877
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: the inner
Suggested change
|
||||||||||
| } | ||||||||||
| case Failure(_) => throw e | ||||||||||
| } | ||||||||||
| // Do not use the outer plan to resolve the star expression | ||||||||||
|
|
||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Design: this
expandhelper is shared with the function-arg /CreateStruct/CreateArray/In/Murmur3Hash/XxHash64callers further down (lines 1969-1995). Only the project-list path (buildExpandedProjectList) exposesProject.output, which is where the ExprId leak you're fixing actually surfaces. The other callers don't need anAliaswrap — and as theselectExcept.sql.outandResolveSubquerySuitediffs show, wrapping them changes the auto-generated column name (the innerAS c1, AS c2gets baked into the surrounding alias'stoPrettySQLbeforeCleanupAliasesruns).Could the wrapping be moved to
buildExpandedProjectList(and similar project-list call sites) so function-arg expansions keep bareOuterReferences? That would scope the change to where the leak actually exists and avoid the column-name regression.