[SPARK-35932][SQL] Support extracting hour/minute/second from timestamp without time zone #33136
Conversation
| case (StringType, target: NumericType) => target | ||
| case (StringType, DateType) => DateType | ||
| case (StringType, TimestampType) => TimestampType | ||
| case (StringType, AnyTimestampType) => TimestampType |
There was a problem hiding this comment.
We need this for the default implicit cast from string to timestamp type. This regression is found on running extract.sql.
|
Test build #140356 has finished for PR 33136 at commit
|
|
Kubernetes integration test starting |
|
Kubernetes integration test status success |
| @@ -189,6 +189,7 @@ object AnsiTypeCoercion extends TypeCoercionBase { | |||
| } | |||
|
|
|||
| case (DateType, TimestampType) => Some(TimestampType) | |||
There was a problem hiding this comment.
shall we support Date -> Timestamp NTZ? in case we have such functions in the future.
There was a problem hiding this comment.
Well we can have it but it is not related to this PR and there is no test case for the change
| } | ||
|
|
||
| case (DateType, TimestampType) => Some(TimestampType) | ||
| case (DateType, AnyTimestampType) => Some(TimestampType) |
There was a problem hiding this comment.
shall we use AnyTimestampType.defaultConcreteType?
There was a problem hiding this comment.
The default concrete type is TimestampWithoutTZType. The current code is not ready for that yet.
There will be a new flag for the default timestamp type in SQL. Let's revisit this later.
There was a problem hiding this comment.
I just change the default concrete type of AnyTimestampType to TimestampType, which makes more sense.
|
Test build #140426 has finished for PR 33136 at commit
|
| if s.left.dataType != s.right.dataType => | ||
| val newLeft = castIfNotSameType(s.left, AnyTimestampType.defaultConcreteType) | ||
| val newRight = castIfNotSameType(s.right, AnyTimestampType.defaultConcreteType) | ||
| val newLeft = castIfNotSameType(s.left, TimestampWithoutTZType) |
There was a problem hiding this comment.
Here the result is the same if casting both to TimestampWithoutTZType or TimestampType. Casting to TimestampWithoutTZType is more straightforward.
|
Kubernetes integration test starting |
|
Kubernetes integration test status success |
|
Merging to master |
|
|
||
| // Implicit cast between date time types | ||
| case (DateType, TimestampType) => TimestampType | ||
| case (DateType, AnyTimestampType) => TimestampType |
There was a problem hiding this comment.
here should be AnyTimestampType.defaultConcreteType as well
There was a problem hiding this comment.
I will address this in the PR for extracting date fields.
|
Test build #140437 has finished for PR 33136 at commit
|
3 participants
What changes were proposed in this pull request?
Support extracting hour/minute/second fields from timestamp without time zone values. In details, the following syntaxes are supported:
Why are the changes needed?
Support basic operations for the new timestamp type.
Does this PR introduce any user-facing change?
No, the timestamp without time zone type is not release yet.
How was this patch tested?
Unit test