[SPARK-51421][SQL] Get seconds of TIME datatype #50525

senthh · 2025-04-06T20:08:31Z

What changes were proposed in this pull request?

This PR adds support for extracting the second component from TIME (TimeType) values in Spark SQL. For example:

scala> spark.sql("SELECT SECOND(TIME'13:59:45.99')").show()
+--------------------------+
|second(TIME '13:59:45.99')|
+--------------------------+
|                        45|
+--------------------------+


scala> spark.sql("select second(cast('12:00:01.123' as time(4)))").show(false)
+-------------------------------------+
|second(CAST(12:00:01.123 AS TIME(4)))|
+-------------------------------------+
|1                                    |
+-------------------------------------+

Why are the changes needed?

Spark previously supported second() for only TIMESTAMP type values. TIME support was missing, leading to implicit casting attempt to TIMESTAMP, which was incorrect. This PR ensures that second(TIME'HH:MM:SS.######') behaves correctly without unnecessary type coercion.

Does this PR introduce any user-facing change?

Yes

Before this PR, calling second(TIME'HH:MM:SS.######') resulted in a type mismatch error or an implicit cast attempt to TIMESTAMP, which was incorrect.
With this PR, second(TIME'HH:MM:SS.######') now works correctly for TIME values without implicit casting.
Users can now extract the second component from TIME values natively.

How was this patch tested?

By running new tests:

$ build/sbt "test:testOnly *TimeExpressionsSuite"

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

No

senthh · 2025-04-06T20:09:34Z

@MaxGekk Could you please review this PR?

HyukjinKwon · 2025-04-07T00:36:22Z

sql/core/src/test/resources/sql-tests/results/hll.sql.out

 -- !query output
-1
+java.lang.NoClassDefFoundError
+Could not initialize class org.apache.datasketches.memory.internal.BaseWritableMemoryImpl


Seems like the output is invalid ..

@HyukjinKwon I have regenerated hll.sql.out

…l.out

vinodkc · 2025-04-07T05:26:36Z

...catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/TimeExpressionsSuite.scala

+  test("Second with TIME type") {
+    // A few test times in microseconds since midnight:
+    //   time in microseconds -> expected second
+    val testTimes = Seq(


Can you please add these tests to test the output based on precision?

val time = "13:10:15.987654321" Seq( 0 -> 15.toDouble, 1 -> 15.9, 2 -> 15.98, 3 -> 15.987, 4 -> 15.9876, 5 -> 15.98765, 6 -> 15.987654).foreach { case (precision, expected) => checkEvaluation( SecondsOfTime(Literal.create(time, TimeType(precision))), BigDecimal(expected)) }

@vinodkc Sure Vinod. I will create a function for SecondsOfTimeWithFraction and also include tests for the same

@senthh , I updated the above comment; please check

vinodkc · 2025-04-07T13:25:54Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/timeExpressions.scala

+
+  override def replacement: Expression = StaticInvoke(
+    classOf[DateTimeUtils.type],
+    IntegerType,


Shouldn't it be DecimalType with a precision and scale that matches the precision of TimeType?

@vinodkc In jira, @MaxGekk has given just IntegerType seconds for an example. So I thought the requirement is just to handle IntegerType. I can modify the implementation so that it should handle both with precision and without.

Hi @vinodkc ,

MaxxGek has responded in jira that we need to return second without fraction. So as per Maxx requirement this PR will work without Fraction.

And also test failed with "java.lang.OutOfMemoryError: Java heap space" is not relevant to our changes

@HyukjinKwon @vinodkc and @MaxGekk It will be helpful if you re-review this PR and provide your input

Yep, the type should be IntergerType

MaxGekk · 2025-04-08T08:41:29Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/timeExpressions.scala

+
+  override def replacement: Expression = StaticInvoke(
+    classOf[DateTimeUtils.type],
+    IntegerType,


Yep, the type should be IntergerType

MaxGekk · 2025-04-08T08:49:00Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/timeExpressions.scala

+    Seq(child.dataType)
+  )
+
+  override def inputTypes: Seq[AbstractDataType] = Seq(TimeType())


You allow any precision of TimeType in SecondExpressionBuilder, but here expectsonly TimeType(6). It should work for any valid precision. For now, see:

spark-sql (default)> select second(cast('12:00:01.123' as time(3))); [DATATYPE_MISMATCH.UNEXPECTED_INPUT_TYPE] Cannot resolve "second(CAST(12:00:01.123 AS TIME(3)))" due to data type mismatch: The first parameter requires the "TIME(6)" type, however "CAST(12:00:01.123 AS TIME(3))" has the type "TIME(3)". SQLSTATE: 42K09; line 1 pos 7;

Now we are able to accept any precision of TimeType.

Query:

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

output:

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

MaxGekk · 2025-04-08T08:50:46Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/timeExpressions.scala

+    Examples:
+      > SELECT _FUNC_('2018-02-14 12:58:59');
+       59
+      > SELECT _FUNC_(TIME'13:59:59.999999');


It would be better to demonstrate different numbers for seconds and minutes, let's say:

> SELECT _FUNC_(TIME'13:10:59.999999');

@MaxGekk Yes good one. I have modified the usage section as below

> SELECT _FUNC_(TIME'13:25:59.999999');

@MaxGekk Could you please re-review the changes?

…ondsOfTime

MaxGekk · 2025-04-09T09:56:41Z

@senthh Could you double check PR's description and replace hour by second, see
"
This PR adds support for extracting the hour component from TIME (TimeType) values in Spark SQL.
"

senthh · 2025-04-09T11:37:33Z

@senthh Could you double check PR's description and replace hour by second, see " This PR adds support for extracting the hour component from TIME (TimeType) values in Spark SQL. "

@MaxGekk Yes Maxx, I have corrected PR description

senthh · 2025-04-09T11:38:54Z

@HyukjinKwon Could you please re-review this PR?

MaxGekk · 2025-04-09T11:45:27Z

@MaxGekk Yes Maxx, I have corrected PR description

Thanks.

+1, LGTM. Merging to master.
Thank you, @senthh and @vinodkc @HyukjinKwon for review.

[SPARK-51421][SQL] Get seconds of TIME datatype

f8811c6

github-actions bot added the SQL label Apr 6, 2025

HyukjinKwon reviewed Apr 7, 2025

View reviewed changes

[SPARK-51421][SQL] Get seconds of TIME datatype - Re-generated hll.sq…

af31525

…l.out

vinodkc reviewed Apr 7, 2025

View reviewed changes

senthh requested review from HyukjinKwon and vinodkc April 8, 2025 07:28

MaxGekk reviewed Apr 8, 2025

View reviewed changes

[SPARK-51462][SQL] Corrected to accept any precision as Input for Sec…

5c7ea7a

…ondsOfTime

senthh requested a review from MaxGekk April 8, 2025 13:37

MaxGekk approved these changes Apr 9, 2025

View reviewed changes

MaxGekk closed this in 5602fbf Apr 9, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-51421][SQL] Get seconds of TIME datatype #50525

[SPARK-51421][SQL] Get seconds of TIME datatype #50525

senthh commented Apr 6, 2025 •

edited by MaxGekk

Loading

senthh commented Apr 6, 2025

HyukjinKwon Apr 7, 2025

senthh Apr 7, 2025

vinodkc Apr 7, 2025 •

edited

Loading

senthh Apr 7, 2025

vinodkc Apr 7, 2025

vinodkc Apr 7, 2025

senthh Apr 7, 2025 •

edited

Loading

senthh Apr 8, 2025

senthh Apr 8, 2025

senthh Apr 8, 2025

MaxGekk Apr 8, 2025

MaxGekk Apr 8, 2025

MaxGekk Apr 8, 2025

senthh Apr 8, 2025

MaxGekk Apr 8, 2025

senthh Apr 8, 2025

senthh Apr 8, 2025

MaxGekk commented Apr 9, 2025

senthh commented Apr 9, 2025

senthh commented Apr 9, 2025

MaxGekk commented Apr 9, 2025

[SPARK-51421][SQL] Get seconds of TIME datatype #50525

[SPARK-51421][SQL] Get seconds of TIME datatype #50525

Conversation

senthh commented Apr 6, 2025 • edited by MaxGekk Loading

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

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

senthh commented Apr 6, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vinodkc Apr 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

senthh Apr 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaxGekk commented Apr 9, 2025

senthh commented Apr 9, 2025

senthh commented Apr 9, 2025

MaxGekk commented Apr 9, 2025

senthh commented Apr 6, 2025 •

edited by MaxGekk

Loading

vinodkc Apr 7, 2025 •

edited

Loading

senthh Apr 7, 2025 •

edited

Loading