[SPARK-54669][SQL] Remove redundant casting in rCTEs #53428
Conversation
dongjoon-hyun
left a comment
dongjoon-hyun
left a comment
There was a problem hiding this comment.
Please file a JIRA issue due to the number of lines, @Pajaraja . 😄
| if (widerType.isDefined && widerType.get == dt) { | ||
| Alias(Cast(attr, dt), attr.name)() | ||
| if (attr.dataType != dt) { | ||
| Alias(Cast(attr, dt, Some(conf.sessionLocalTimeZone)), attr.name)() |
There was a problem hiding this comment.
This is non-trivial because you are adding Some(conf.sessionLocalTimeZone).
There was a problem hiding this comment.
I added a test with time sensitive CAST in the golden file, but I think it's okay either way since ResolveTimeZone is in the same batch as TypeCoercion.
I changed it here to have parity with how Union/Intersect/Except do it, but it should be no-op.
Pajaraja
changed the title
|
Added a Jira ticket |
dongjoon-hyun
changed the title
| SELECT n+1, stamp FROM t1 WHERE n < 5) | ||
| SELECT * FROM t1 | ||
| -- !query analysis | ||
| [Analyzer test output redacted due to nondeterminism] |
There was a problem hiding this comment.
Why is this non-deterministic? If it is really non-deterministic, can you add a deterministic test?
There was a problem hiding this comment.
It was nondeterministic because the original relation was Date. I changed into string now.
It's deterministic now.
peter-toth
left a comment
peter-toth
left a comment
There was a problem hiding this comment.
LGTM. Maybe we could simplify the new test a bit, but that's just a nit.
|
thanks, merging to master! |
4 participants
What changes were proposed in this pull request?
Change TypeCoercionBase so that the only outputs that are casted are the ones that need to be casted.
Why are the changes needed?
Remove redundant CAST.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Existing golden file test cte-recursion.
Was this patch authored or co-authored using generative AI tooling?
No.