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-31797][SQL][FOLLOWUP] TIMESTAMP_SECONDS supports fractional input #28956
Conversation
@@ -1142,28 +1142,6 @@ class DateExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper { | |||
} | |||
} | |||
|
|||
test("SPARK-31710:Adds TIMESTAMP_SECONDS, " + |
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 test is rewritten to increase test coverage.
Test build #124646 has finished for PR 28956 at commit
|
extends NumberToTimestampBase { | ||
// scalastyle:on line.size.limit | ||
case class SecondsToTimestamp(child: Expression) extends UnaryExpression | ||
with ExpectsInputTypes with NullIntolerant{ |
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.
nit: NullIntolerant{
-> NullIntolerant {
.
@@ -2,13 +2,16 @@ | |||
|
|||
-- [SPARK-31710] TIMESTAMP_SECONDS, TIMESTAMP_MILLISECONDS and TIMESTAMP_MICROSECONDS to timestamp transfer | |||
select TIMESTAMP_SECONDS(1230219000),TIMESTAMP_SECONDS(-1230219000),TIMESTAMP_SECONDS(null); | |||
select TIMESTAMP_SECONDS(1.23), TIMESTAMP_SECONDS(1.23d); |
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.
Since this has Decimal
and Double
, can we have Float
together by using TIMESTAMP_SECONDS(CAST(1.23 AS FLOAT))
?
select TIMESTAMP_SECONDS(1230219000123123); | ||
select TIMESTAMP_SECONDS(-1230219000123123); | ||
select TIMESTAMP_MILLIS(92233720368547758); | ||
select TIMESTAMP_MILLIS(-92233720368547758); | ||
-- truncate exception | ||
select TIMESTAMP_SECONDS(0.1234567); |
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.
Shall we have a test case for allow truncation
together because this PR allows truncation for double
type?
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 PR aims to allow truncation for both ANSI and legacy mode. Did I understand correctly?
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.
yes, float/double are approximate values and truncation should be always allowed.
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.
+1, LGTM. Thank you for updating, @cloud-fan .
Test build #124747 has finished for PR 28956 at commit
|
Merged to master for Apache Spark 3.1.0. |
+1 looks good to me too. |
What changes were proposed in this pull request?
This is a followup of #28534 , to make
TIMESTAMP_SECONDS
function support fractional input as well.Why are the changes needed?
Previously the cast function can cast fractional values to timestamp. Now we suggest users to ues these new functions, and we need to cover all the cast use cases.
Does this PR introduce any user-facing change?
Yes, now
TIMESTAMP_SECONDS
function accepts fractional input.How was this patch tested?
new tests