Support TIMESTAMP_SECONDS, TIMESTAMP_MILLIS and TIMESTAMP_MICROS [databricks] #8650
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
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
firestarman
reviewed
Jul 5, 2023
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala
Outdated
Show resolved
Hide resolved
Signed-off-by: Haoyang Li <[email protected]>
|
@firestarman Thanks for review! I think I have addressed all your comments. |
|
build |
revans2
reviewed
Jul 5, 2023
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala
Show resolved
Hide resolved
| } | ||
| case DoubleType | FloatType => | ||
| (input: GpuColumnVector) => { | ||
| // basicly copied from GpuCast |
There was a problem hiding this comment.
If this was copied from GpuCast would it be simpler to just use GpuCast for this??
GpuCast.castTo(input, input.dataType, TimestampType, false, false, false)
Is it because of the shim hasCastFloatTimestampUpcast?
There was a problem hiding this comment.
Is it because of the shim hasCastFloatTimestampUpcast?
Yes, but spark#31831 added the upcast at the same version as the hasCastFloatTimestampUpcast from false to true, so GpuCast can be called here safely. I didn't notice that, updated.
| # (-62135510400, 253402214400) is the range of seconds that can be represented by timestamp_seconds | ||
| # considering the influence of time zone. | ||
| seconds_gens = [LongGen(min_val=-62135510400, max_val=253402214400), IntegerGen(), ShortGen(), ByteGen(), | ||
| DoubleGen(min_exp=0, max_exp=32), ts_float_gen, DecimalGen(16, 6), DecimalGen(13, 3), DecimalGen(10, 0), DecimalGen(6, 6)] |
There was a problem hiding this comment.
Can we test negative scale decimal values?
| } | ||
| case dt: DecimalType => | ||
| (input: GpuColumnVector) => { | ||
| // SecondsToTimestamp only supports decimals with a scale of 6 or less, which can be |
There was a problem hiding this comment.
This is confusing. It indicates that Spark's SecondsToTimestamp does not support decimals with a scaled of 6 or less. But it does. You even test it.
There was a problem hiding this comment.
SecondsToTimestamp does support decimals with a scale of 6 or less, which can be represented as microseconds. An exception will be thrown if the scale is more than 6. I updated the comment here.
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
|
build |
firestarman
reviewed
Jul 6, 2023
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala
Show resolved
Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala
Outdated
Show resolved
Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala
Show resolved
Hide resolved
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala
Outdated
Show resolved
Hide resolved
Signed-off-by: Haoyang Li <[email protected]>
|
@firestarman Thanks for the review. All done, please take another look. |
|
build |
revans2
previously approved these changes
Jul 6, 2023
2 tasks
firestarman
reviewed
Jul 7, 2023
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/datetimeExpressions.scala
Outdated
Show resolved
Hide resolved
|
Looks good to me, only one nit |
Signed-off-by: Haoyang Li <[email protected]>
thirtiseven
changed the title
thirtiseven
changed the title
firestarman
previously approved these changes
Jul 7, 2023
|
build |
Signed-off-by: Haoyang Li <[email protected]>
firestarman
approved these changes
Jul 7, 2023
|
build |
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.
Closes #308
This PR adds GPU support for functions TIMESTAMP_SECONDS, TIMESTAMP_MILLIS, and TIMESTAMP_MICROS, which are used to convert the number of seconds/milliseconds/microseconds from the Unix epoch to a timestamp.
Related PRs in Spark: 28534 and 28956.