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
[SPARK-48252][SQL] Update CommonExpressionRef when necessary #46552
Conversation
cc @viirya |
...lyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/RewriteWithExpressionSuite.scala
Outdated
Show resolved
Hide resolved
if (newDef.dataType != oldDef.dataType || newDef.nullable != oldDef.nullable) { | ||
val newRef = new CommonExpressionRef(newDef) | ||
result.transform { | ||
case oldRef: CommonExpressionRef if oldRef.id == newRef.id => |
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.
Nit: We can use oldRef.id == newDFef.id to avoid creating some newRef?
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.
unless there is dangling common expressions (it should not happen today), we should aways create a new ref when we know the def is different.
Hi, @cloud-fan . It seems that @viirya 's comment is marked |
oh, I accepted this code-change comment on Github but later on I pushed a new commit from local which discarded this change... |
…mizer/RewriteWithExpressionSuite.scala Co-authored-by: Liang-Chi Hsieh <viirya@gmail.com>
The last commit just updates the test name, we don't need to re-test. I'm merging it to master, thanks for the review! |
What changes were proposed in this pull request?
The
With
expression assumes that it should be created after all input expressions are fully resolved. This is mostly true (function lookup happens after function input expressions are resolved), but there is a special case of column resolution in HAVING: we useTempResolvedColumn
to try one column resolution option. If it doesn't work, re-resolve the column, which may be a different data type.With
expression should update the refs when this happens.Why are the changes needed?
bug fix, otherwise the query will fail
Does this PR introduce any user-facing change?
This feature is not released yet.
How was this patch tested?
new test
Was this patch authored or co-authored using generative AI tooling?
no