-
Notifications
You must be signed in to change notification settings - Fork 28.3k
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-49022][CONNECT][SQL][FOLLOW-UP] Parse unresolved identifier to keep the behavior same #48376
Conversation
cc @hvanhovell |
4e6ccd9
to
b27c4c1
Compare
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/Dataset.scala
Outdated
Show resolved
Hide resolved
...ctor/connect/client/jvm/src/main/scala/org/apache/spark/sql/internal/columnNodeSupport.scala
Outdated
Show resolved
Hide resolved
@@ -74,7 +74,7 @@ private[sql] trait ColumnNodeToExpressionConverter extends (ColumnNode => Expres | |||
analysis.UnresolvedRegex(columnNameRegex, Some(nameParts), conf.caseSensitiveAnalysis) | |||
|
|||
case UnresolvedRegex(unparsedIdentifier, planId, _) => | |||
convertUnresolvedAttribute(unparsedIdentifier, planId, isMetadataColumn = false) | |||
convertUnresolvedRegex(unparsedIdentifier, planId) |
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.
This seems a bit inconsistent with unresolved attributes. Is this observable?
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 it's fine in a way that it's consistent with catalyst ..
a05df9b
to
8e586a5
Compare
f4580b8
to
2ad70f4
Compare
Let me merge this @hvanhovell please let me know if there are some more things to change, I will make another PR. |
Merged to master. |
What changes were proposed in this pull request?
This PR is a followup of #47688 that keeps
Column.toString
as the same before.Why are the changes needed?
To keep the same behaviour with Spark Classic and Connect.
Does this PR introduce any user-facing change?
No, the main change has not been released out yet.
How was this patch tested?
Will be added separately. I manually tested:
Was this patch authored or co-authored using generative AI tooling?
No.