Skip to content

[SPARK-51419][SQL][FOLLOWUP] Making hour function to accept any precision of TIME type #50554

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

Closed
wants to merge 2 commits into from

Conversation

senthh
Copy link
Contributor

@senthh senthh commented Apr 10, 2025

What changes were proposed in this pull request?

This is followup PR of SPARK-51419

Why are the changes needed?

This Followup PR allows any precision in the range of [0,6] for the hour function

Does this PR introduce any user-facing change?

Yes. This changes allows User to execute below query

spark.sql("select hour(cast('12:00:01.123' as time(3)))").show(false)

How was this patch tested?

We tested by running sample query as below

spark.sql("select hour(cast('12:00:01.123' as time(3)))").show(false)

Output:

+-----------------------------------+
|hour(CAST(12:00:01.123 AS TIME(3)))|
+-----------------------------------+
|12                                                        |
+-----------------------------------+

Was this patch authored or co-authored using generative AI tooling?

No

@github-actions github-actions bot added the SQL label Apr 10, 2025
@senthh
Copy link
Contributor Author

senthh commented Apr 10, 2025

@MaxGekk Could you please review this Follow-up PR of SPARK-51419 ?

@senthh
Copy link
Contributor Author

senthh commented Apr 10, 2025

Both test failures are not relevant with this changes

[info] - flatMapGroupsWithState - initial state and initial batch have same keys and skipEmittingInitialStateKeys=false - state format version 1 *** FAILED *** (690 milliseconds)
[info]   == Results ==
[info]   !== Correct Answer - 2 ==   == Spark Answer - 4 ==

and

 SparkSessionE2ESuite:
[info] - interrupt all - background queries, foreground interrupt *** FAILED *** (20 seconds, 53 milliseconds)
[info]   The code passed to eventually never returned normally. Attempted 30 times over 20.049380065999998 seconds. Last failure message: q1Interrupted was false. (SparkSessionE2ESuite.scala:72)
[info]   org.scalatest.exceptions.TestFailedDueToTimeoutException:

Copy link
Member

@MaxGekk MaxGekk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a check for the input types, see for instance #50551

@senthh
Copy link
Contributor Author

senthh commented Apr 11, 2025

Could you add a check for the input types, see for instance #50551

Yes @MaxGekk I have added test as you mentioned

@senthh senthh requested a review from MaxGekk April 11, 2025 18:26
@MaxGekk
Copy link
Member

MaxGekk commented Apr 11, 2025

I think the test failure is not related to the changes:

[info] - SPARK-41224: collect data using arrow *** FAILED *** (33 milliseconds)
[info]   VerifyEvents.this.executeHolder.eventsManager.hasError.isDefined was false (SparkConnectServiceSuite.scala:895)
[info]   org.scalatest.exceptions.TestFailedException:
[info]   at org.scalatest.Assertions.newAssertionFailedException(Assertions.scala:472

@MaxGekk
Copy link
Member

MaxGekk commented Apr 11, 2025

+1, LGTM. Merging to master.
Thank you, @senthh.

@MaxGekk MaxGekk closed this in b9dbf8b Apr 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants