-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-47713][SQL][CONNECT] Fix a self-join failure #45846
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-47713][SQL][CONNECT] Fix a self-join failure #45846
Conversation
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/ClientE2ETestSuite.scala
Outdated
Show resolved
Hide resolved
ffcbae7 to
0124dec
Compare
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/ClientE2ETestSuite.scala
Outdated
Show resolved
Hide resolved
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ColumnResolutionHelper.scala
Outdated
Show resolved
Hide resolved
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ColumnResolutionHelper.scala
Outdated
Show resolved
Hide resolved
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.
when this condition can be false if resolved.isEmpty is true?
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.
I think there are two cases:
1, analyzer rules supporting missing column resolution
2, plan id missed in some way (should be bugs)
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.
Shall we still have the matched flag? The new code is confusing and I can't understand it even after reading your comment.
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.
A comment in code would be very helpful
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.
ok, let me restore matched
923f046 to
b400131
Compare
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.
b400131 to
3aa147e
Compare
fix scala style
3aa147e to
eda211c
Compare
|
thanks, merging to master! |

What changes were proposed in this pull request?
update the logic to resolve column in spark connect
Why are the changes needed?
Does this PR introduce any user-facing change?
yes, above query can run successfully after this PR
This PR only affects Spark Connect, won't affect Classic Spark.
How was this patch tested?
added tests
Was this patch authored or co-authored using generative AI tooling?
no