-
Notifications
You must be signed in to change notification settings - Fork 28.7k
[SPARK-51554][SQL] Add the time_trunc() function for TIME datatype #50607
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
base: master
Are you sure you want to change the base?
Conversation
@MaxGekk While I am trying to convert this into a |
@MaxGekk Please do let me know your thoughts on this one! Have updated the revision. |
@MaxGekk Any chance I can get a review here, please? |
Hey @MaxGekk , could I get a review here, please? Looking forward to the comments. |
@@ -351,7 +351,7 @@ case class SecondsOfTime(child: Expression) | |||
) | |||
|
|||
override def inputTypes: Seq[AbstractDataType] = | |||
Seq(TypeCollection(TimeType.MIN_PRECISION to TimeType.MAX_PRECISION map TimeType.apply: _*)) | |||
Seq(TypeCollection(TimeType.MIN_PRECISION to TimeType.MAX_PRECISION map TimeType: _*)) |
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.
Please, revert it back otherwise it causes a compilation warning, see #50692
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.
Sure will do!
if (level < MIN_LEVEL_OF_TIME_TRUNC) { // unknown unit | ||
-1L | ||
} else { | ||
val out = truncTime(micros, level) // may still return -1L on error |
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.
What's the reason of splitting the function? Is truncTime
used somewhere else?
override def inputTypes: Seq[AbstractDataType] = | ||
Seq( | ||
StringType, | ||
TypeCollection(TimeType.MIN_PRECISION to TimeType.MAX_PRECISION map TimeType: _*) |
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.
Could you double check that it won't produce warnings.
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.
Will check!
What changes were proposed in this pull request?
time_trunc(unit, expr)
that returns aTIME
value truncated to the specified unit.TIME
type or a string that can be cast to TIME.HOUR
,MINUTE
,SECOND
,MILLISECOND
, andMICROSECOND
.Why are the changes needed?
truncTimestamp
.Does this PR introduce any user-facing change?
Yes. A new built-in function
time_trunc
is added. Users can call the function to truncate TIME values to one of the above mentioned supported units.How was this patch tested?
By running newly added UTs:
$ build/sbt "test:testOnly *TimeExpressionsSuite.scala"
By manual tests:
Was this patch authored or co-authored using generative AI tooling?
No.