From 4c282c72598e192be4d8e13691f50312e70e68fb Mon Sep 17 00:00:00 2001 From: yangjie01 Date: Thu, 23 Jul 2026 22:55:16 +0800 Subject: [PATCH] [SPARK-58296][SQL] Fix to_time returning STRING type when the format is a foldable NULL `ToTime` is a `RuntimeReplaceable`, so its `dataType` comes from `replacement.dataType`. When the format argument is foldable and evaluates to NULL, the replacement fell back to `Literal(null, expr.dataType)` where `expr` is the format argument, making `to_time` report STRING instead of TIME. Change the fallback to `Literal(null, TimeType())` so it matches every other branch. --- .../catalyst/expressions/timeExpressions.scala | 2 +- .../expressions/TimeExpressionsSuite.scala | 14 ++++++++++++++ .../sql-tests/analyzer-results/time.sql.out | 14 ++++++++++++++ .../src/test/resources/sql-tests/inputs/time.sql | 4 ++++ .../resources/sql-tests/results/time.sql.out | 16 ++++++++++++++++ 5 files changed, 49 insertions(+), 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/timeExpressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/timeExpressions.scala index ab0cde5523741..dc91dd672a368 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/timeExpressions.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/timeExpressions.scala @@ -98,7 +98,7 @@ case class ToTime(str: Expression, format: Option[Expression]) case Some(expr) if expr.foldable => Option(expr.eval()) .map(f => invokeParser(Some(f.toString), Seq(str))) - .getOrElse(Literal(null, expr.dataType)) + .getOrElse(Literal(null, TimeType())) case _ => invokeParser() } diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/TimeExpressionsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/TimeExpressionsSuite.scala index e03d8cb39ea2d..f8f34ae9d2c76 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/TimeExpressionsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/TimeExpressionsSuite.scala @@ -59,6 +59,20 @@ class TimeExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper { parameters = Map("input" -> "'100:50'", "format" -> "'mm:HH'")) } + test("SPARK-58296: to_time reports TimeType even when a foldable format is NULL") { + // A foldable format that evaluates to NULL must not change the output type: to_time + // still produces a TIME, matching the non-null and no-format branches. Before the fix + // the replacement fell back to the format argument's own type (STRING/NULL). + assert(new ToTime(Literal("00:00:00"), Literal.create(null, StringType)).dataType === + TimeType()) + assert(new ToTime(Literal("00:00:00"), Literal.create(null)).dataType === TimeType()) + // The value is still NULL; assert type and value together for this exact case. + checkEvaluation(new ToTime(Literal("00:00:00"), Literal.create(null, StringType)), null) + // Sanity: the branches that were already correct. + assert(new ToTime(Literal("00:00:00")).dataType === TimeType()) + assert(new ToTime(Literal("00:00:00"), Literal("HH:mm:ss")).dataType === TimeType()) + } + test("HourExpressionBuilder") { // Empty expressions list checkError( diff --git a/sql/core/src/test/resources/sql-tests/analyzer-results/time.sql.out b/sql/core/src/test/resources/sql-tests/analyzer-results/time.sql.out index d5d5700316f86..3b0cece05ea6d 100644 --- a/sql/core/src/test/resources/sql-tests/analyzer-results/time.sql.out +++ b/sql/core/src/test/resources/sql-tests/analyzer-results/time.sql.out @@ -48,6 +48,20 @@ Project [to_time(time_str#x, Some(fmt_str#x)) AS to_time(time_str, fmt_str)#x] +- OneRowRelation +-- !query +select typeof(to_time('00:12:00', null)) +-- !query analysis +Project [typeof(to_time(00:12:00, Some(null))) AS typeof(to_time(00:12:00, NULL))#x] ++- OneRowRelation + + +-- !query +select typeof(to_time('00:12:00', cast(null as string))) +-- !query analysis +Project [typeof(to_time(00:12:00, Some(cast(null as string)))) AS typeof(to_time(00:12:00, CAST(NULL AS STRING)))#x] ++- OneRowRelation + + -- !query select to_time("11", "HH") -- !query analysis diff --git a/sql/core/src/test/resources/sql-tests/inputs/time.sql b/sql/core/src/test/resources/sql-tests/inputs/time.sql index 1a836b270edc0..ee27d2acc1f9f 100644 --- a/sql/core/src/test/resources/sql-tests/inputs/time.sql +++ b/sql/core/src/test/resources/sql-tests/inputs/time.sql @@ -11,6 +11,10 @@ select time '16:39:45\t'; select to_time(null), to_time('01:02:03'), to_time('23-59-59.999999', 'HH-mm-ss.SSSSSS'); select to_time(time_str, fmt_str) from time_view; +-- SPARK-58296: a NULL format must not change the result type of `to_time` (stays TIME). +select typeof(to_time('00:12:00', null)); +select typeof(to_time('00:12:00', cast(null as string))); + -- missing fields in `to_time` select to_time("11", "HH"); -- invalid: there is no 13 hours diff --git a/sql/core/src/test/resources/sql-tests/results/time.sql.out b/sql/core/src/test/resources/sql-tests/results/time.sql.out index 31444a600b492..58e72aec919e9 100644 --- a/sql/core/src/test/resources/sql-tests/results/time.sql.out +++ b/sql/core/src/test/resources/sql-tests/results/time.sql.out @@ -47,6 +47,22 @@ struct 11:53:26.038344 +-- !query +select typeof(to_time('00:12:00', null)) +-- !query schema +struct +-- !query output +time(6) + + +-- !query +select typeof(to_time('00:12:00', cast(null as string))) +-- !query schema +struct +-- !query output +time(6) + + -- !query select to_time("11", "HH") -- !query schema