Skip to content

[SPARK-45069][SQL] SQL variable should always be resolved after outer reference#42803

Closed
cloud-fan wants to merge 1 commit intoapache:masterfrom
cloud-fan:meta-col
Closed

[SPARK-45069][SQL] SQL variable should always be resolved after outer reference#42803
cloud-fan wants to merge 1 commit intoapache:masterfrom
cloud-fan:meta-col

Conversation

@cloud-fan
Copy link
Contributor

What changes were proposed in this pull request?

This is a bug fix for the recently added SQL variable feature. It's designed to resolve columns to SQL variable as the last resort, but for columns in Aggregate, we may resolve columns to outer reference first.

Why are the changes needed?

bug fix

Does this PR introduce any user-facing change?

yes, the query result can be wrong before this fix

How was this patch tested?

new tests

Was this patch authored or co-authored using generative AI tooling?

no

@github-actions github-actions bot added the SQL label Sep 4, 2023
try {
val resolved = innerResolve(expr, isTopLevel = true)
val withOuterResolved = if (allowOuter) resolveOuterRef(resolved) else resolved
resolveVariables(withOuterResolved)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the bug. Sometimes we only want to do basic column resolution first, and then try some special resolution logic (see ResolveReferencesInAggregat as an example), and finally resolve outer references. The code before always tried to resolve the SQL variable.

To avoid hitting similar bugs, I refactored the code a bit to have a last resort step for column resolution, which contains resolving outer references and SQL variables.

@cloud-fan
Copy link
Contributor Author

cc @srielau @MaxGekk

@MaxGekk
Copy link
Member

MaxGekk commented Sep 11, 2023

@cloud-fan We shouldn't backport this to 3.5, correct?

@cloud-fan
Copy link
Contributor Author

@MaxGekk no, sql variable is only in the master branch.

@cloud-fan
Copy link
Contributor Author

thanks for the review, merging to master!

@cloud-fan cloud-fan closed this in 2a10c8d Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants