[SPARK-51310][SQL] Resolve the type of default string producing expressions #50053
+87
−2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HyukjinKwon
reviewed
Feb 24, 2025
...yst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveDDLCommandStringTypes.scala
Outdated
Show resolved
Hide resolved
stefankandic
changed the title
…out adding node tag
stefankandic
changed the title
|
@cloud-fan @dejankrak-db please take a look, thanks! |
dejankrak-db
approved these changes
Feb 25, 2025
| @@ -28,6 +28,7 @@ import org.apache.spark.sql.types.{DataType, StringType} | |||
| * collation from the corresponding object (table/view -> schema -> catalog). | |||
| */ | |||
| object ResolveDDLCommandStringTypes extends Rule[LogicalPlan] { | |||
|
|
|||
There was a problem hiding this comment.
nit: you can remove this extra line now
dejankrak-db
approved these changes
Feb 25, 2025
cloud-fan
reviewed
Feb 26, 2025
|
|
||
| plan.mapChildren { operator => | ||
| operator.mapExpressions { expression => | ||
| expression.mapChildren { |
There was a problem hiding this comment.
Does .mapChildren transform the whole expression tree?
There was a problem hiding this comment.
There was a problem hiding this comment.
if (containsChild.nonEmpty) {
withNewChildren(children.map(f))
} else {
this
}
There is no recursion, how does it transform the full tree?
There was a problem hiding this comment.
Yeah, you are right, sorry I missed this. Changed the behavior to visit recursively and added new test to verify this behavior.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Add casts to
DefaultStringProducingExpressionto have the correct type based on the default string type.Currently, we can only test this on views, until we implement schema level collations when we will be able to test the behavior on newly created tables as well.
Why are the changes needed?
These expressions should return the default string type, which could be different in some DDL commands.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Added unit tests for view creation.
Was this patch authored or co-authored using generative AI tooling?
No.