-
Notifications
You must be signed in to change notification settings - Fork 28.5k
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-51442][SQL] Add time formatters #50190
Conversation
TimeFormatter
@LuciferYang @gengliangwang Could you review this PR, please. |
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
Thanks @MaxGekk
with DateTimeFormatterHelper { | ||
|
||
@transient | ||
protected lazy val formatter = getOrCreateFormatter(pattern, locale, isParsing) |
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: for a protected
field, readability might be improved if a type declaration could be added.
isParsing = false) { | ||
|
||
@transient | ||
override protected lazy val formatter = DateTimeFormatterHelper.fractionTimeFormatter |
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.
ditto
@@ -112,4 +112,15 @@ object DateTimeTestUtils { | |||
result = Math.addExact(result, Math.multiplyExact(seconds, MICROS_PER_SECOND)) | |||
result | |||
} | |||
|
|||
// Returns microseconds since midnight | |||
def localtime( |
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.
def localtime
-> def localTime
Merging to master. Thank you, @LuciferYang for review. |
### What changes were proposed in this pull request? In the PR, I propose two new formatters: `Iso8601TimeFormatter` and `FractionTimeFormatter` similar to the date or timestamp formatters: - The ISO 8601 formatter is capable of formatting and parsing the ISO-8601 extended time format, - The fraction formatter is similar to the previous one but does not output trailing zeros in the fraction. ### Why are the changes needed? The datetime formatters are used in `CAST` and other parsing/formatting expressions. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? By running new test suite: ``` $ build/sbt "test:testOnly *TimeFormatterSuite" $ build/sbt "test:testOnly *TimestampFormatterSuite" ``` ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#50190 from MaxGekk/time-formatter. Authored-by: Max Gekk <[email protected]> Signed-off-by: Max Gekk <[email protected]>
### What changes were proposed in this pull request? In the PR, I propose two new formatters: `Iso8601TimeFormatter` and `FractionTimeFormatter` similar to the date or timestamp formatters: - The ISO 8601 formatter is capable of formatting and parsing the ISO-8601 extended time format, - The fraction formatter is similar to the previous one but does not output trailing zeros in the fraction. ### Why are the changes needed? The datetime formatters are used in `CAST` and other parsing/formatting expressions. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? By running new test suite: ``` $ build/sbt "test:testOnly *TimeFormatterSuite" $ build/sbt "test:testOnly *TimestampFormatterSuite" ``` ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#50190 from MaxGekk/time-formatter. Authored-by: Max Gekk <[email protected]> Signed-off-by: Max Gekk <[email protected]>
What changes were proposed in this pull request?
In the PR, I propose two new formatters:
Iso8601TimeFormatter
andFractionTimeFormatter
similar to the date or timestamp formatters:Why are the changes needed?
The datetime formatters are used in
CAST
and other parsing/formatting expressions.Does this PR introduce any user-facing change?
No.
How was this patch tested?
By running new test suite:
Was this patch authored or co-authored using generative AI tooling?
No.