From 20f8f3a03c790c7f901e52978dcb961fefd27396 Mon Sep 17 00:00:00 2001 From: Your Name Date: Mon, 9 Feb 2026 14:15:35 +0800 Subject: [PATCH 1/7] [Feature] Support Spark expression: minutes_of_time --- .../apache/comet/shims/CometExprShim.scala | 22 ++++++++++++++++++- .../apache/comet/CometExpressionSuite.scala | 17 ++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala b/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala index 1d4427d159..a17e81f624 100644 --- a/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala +++ b/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala @@ -19,7 +19,7 @@ package org.apache.comet.shims -import org.apache.spark.sql.catalyst.expressions._ +import org.apache.spark.sql.catalyst.expressions.{Expression, MinutesOfTime} import org.apache.spark.sql.catalyst.expressions.objects.StaticInvoke import org.apache.spark.sql.internal.SQLConf import org.apache.spark.sql.internal.types.StringTypeWithCollation @@ -108,6 +108,26 @@ trait CometExprShim extends CommonStringExprs { val optExpr = scalarFunctionExprToProto("width_bucket", childExprs: _*) optExprWithInfo(optExpr, wb, wb.children: _*) + case mot: MinutesOfTime => + // MinutesOfTime is a RuntimeReplaceable expression that delegates to DateTimeUtils.getMinutesOfTime. + // It has the same functionality as Minute, so we convert it to the same protobuf Minute message. + val childExpr = exprToProtoInternal(mot.children.head, inputs, binding) + childExpr match { + case Some(child) => + val builder = ExprOuterClass.Minute.newBuilder() + builder.setChild(child) + // RuntimeReplaceable expressions typically don't have timeZoneId, default to UTC + builder.setTimezone("UTC") + Some( + ExprOuterClass.Expr + .newBuilder() + .setMinute(builder) + .build()) + case None => + withInfo(mot, mot.children.head) + None + } + case _ => None } } diff --git a/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala b/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala index 2999d8bfe5..399d5bb5df 100644 --- a/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala +++ b/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala @@ -618,6 +618,23 @@ class CometExpressionSuite extends CometTestBase with AdaptiveSparkPlanHelper { } } + test("MinutesOfTime expression support") { + // MinutesOfTime is only available in Spark 4.0+ + assume(isSpark40Plus, "MinutesOfTime is only available in Spark 4.0+") + Seq(true, false).foreach { dictionaryEnabled => + withTempDir { dir => + val path = new Path(dir.toURI.toString, "part-r-0.parquet") + makeRawTimeParquetFile(path, dictionaryEnabled = dictionaryEnabled, 10000) + readParquetFile(path.toString) { df => + // Test that MinutesOfTime (via minute() function) works correctly + val query = df.select(expr("minute(_1)")) + + checkSparkAnswerAndOperator(query) + } + } + } + } + test("hour on int96 timestamp column") { import testImplicits._ From 11cce9a41325aa073a672f7c1625c876d37d2be5 Mon Sep 17 00:00:00 2001 From: Your Name Date: Mon, 9 Feb 2026 14:55:01 +0800 Subject: [PATCH 2/7] Revert "[Feature] Support Spark expression: minutes_of_time" This reverts commit 20f8f3a03c790c7f901e52978dcb961fefd27396. --- .../apache/comet/shims/CometExprShim.scala | 22 +------------------ .../apache/comet/CometExpressionSuite.scala | 17 -------------- 2 files changed, 1 insertion(+), 38 deletions(-) diff --git a/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala b/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala index a17e81f624..1d4427d159 100644 --- a/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala +++ b/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala @@ -19,7 +19,7 @@ package org.apache.comet.shims -import org.apache.spark.sql.catalyst.expressions.{Expression, MinutesOfTime} +import org.apache.spark.sql.catalyst.expressions._ import org.apache.spark.sql.catalyst.expressions.objects.StaticInvoke import org.apache.spark.sql.internal.SQLConf import org.apache.spark.sql.internal.types.StringTypeWithCollation @@ -108,26 +108,6 @@ trait CometExprShim extends CommonStringExprs { val optExpr = scalarFunctionExprToProto("width_bucket", childExprs: _*) optExprWithInfo(optExpr, wb, wb.children: _*) - case mot: MinutesOfTime => - // MinutesOfTime is a RuntimeReplaceable expression that delegates to DateTimeUtils.getMinutesOfTime. - // It has the same functionality as Minute, so we convert it to the same protobuf Minute message. - val childExpr = exprToProtoInternal(mot.children.head, inputs, binding) - childExpr match { - case Some(child) => - val builder = ExprOuterClass.Minute.newBuilder() - builder.setChild(child) - // RuntimeReplaceable expressions typically don't have timeZoneId, default to UTC - builder.setTimezone("UTC") - Some( - ExprOuterClass.Expr - .newBuilder() - .setMinute(builder) - .build()) - case None => - withInfo(mot, mot.children.head) - None - } - case _ => None } } diff --git a/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala b/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala index 399d5bb5df..2999d8bfe5 100644 --- a/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala +++ b/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala @@ -618,23 +618,6 @@ class CometExpressionSuite extends CometTestBase with AdaptiveSparkPlanHelper { } } - test("MinutesOfTime expression support") { - // MinutesOfTime is only available in Spark 4.0+ - assume(isSpark40Plus, "MinutesOfTime is only available in Spark 4.0+") - Seq(true, false).foreach { dictionaryEnabled => - withTempDir { dir => - val path = new Path(dir.toURI.toString, "part-r-0.parquet") - makeRawTimeParquetFile(path, dictionaryEnabled = dictionaryEnabled, 10000) - readParquetFile(path.toString) { df => - // Test that MinutesOfTime (via minute() function) works correctly - val query = df.select(expr("minute(_1)")) - - checkSparkAnswerAndOperator(query) - } - } - } - } - test("hour on int96 timestamp column") { import testImplicits._ From feed44f96b4840c70028271e4359c8cf33493746 Mon Sep 17 00:00:00 2001 From: Your Name Date: Mon, 9 Feb 2026 15:05:40 +0800 Subject: [PATCH 3/7] [Feature] Support Spark expression: minutes_of_time --- .../apache/comet/serde/QueryPlanSerde.scala | 1 + .../org/apache/comet/serde/datetime.scala | 28 ++++++++++++++++++- .../apache/comet/CometExpressionSuite.scala | 15 ++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala b/spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala index 73b88ae935..ea4b61a700 100644 --- a/spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala +++ b/spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala @@ -196,6 +196,7 @@ object QueryPlanSerde extends Logging with CometExprShim { classOf[LastDay] -> CometLastDay, classOf[Hour] -> CometHour, classOf[Minute] -> CometMinute, + classOf[MinutesOfTime] -> CometMinutesOfTime, classOf[Second] -> CometSecond, classOf[TruncDate] -> CometTruncDate, classOf[TruncTimestamp] -> CometTruncTimestamp, diff --git a/spark/src/main/scala/org/apache/comet/serde/datetime.scala b/spark/src/main/scala/org/apache/comet/serde/datetime.scala index a623146916..63f3d9cbc5 100644 --- a/spark/src/main/scala/org/apache/comet/serde/datetime.scala +++ b/spark/src/main/scala/org/apache/comet/serde/datetime.scala @@ -21,7 +21,7 @@ package org.apache.comet.serde import java.util.Locale -import org.apache.spark.sql.catalyst.expressions.{Attribute, DateAdd, DateDiff, DateFormatClass, DateSub, DayOfMonth, DayOfWeek, DayOfYear, GetDateField, Hour, LastDay, Literal, Minute, Month, Quarter, Second, TruncDate, TruncTimestamp, UnixDate, UnixTimestamp, WeekDay, WeekOfYear, Year} +import org.apache.spark.sql.catalyst.expressions.{Attribute, DateAdd, DateDiff, DateFormatClass, DateSub, DayOfMonth, DayOfWeek, DayOfYear, GetDateField, Hour, LastDay, Literal, Minute, MinutesOfTime, Month, Quarter, Second, TruncDate, TruncTimestamp, UnixDate, UnixTimestamp, WeekDay, WeekOfYear, Year} import org.apache.spark.sql.types.{DateType, IntegerType, StringType, TimestampType} import org.apache.spark.unsafe.types.UTF8String @@ -228,6 +228,32 @@ object CometMinute extends CometExpressionSerde[Minute] { } } +object CometMinutesOfTime extends CometExpressionSerde[MinutesOfTime] { + override def convert( + expr: MinutesOfTime, + inputs: Seq[Attribute], + binding: Boolean): Option[ExprOuterClass.Expr] = { + val childExpr = exprToProtoInternal(expr.children.head, inputs, binding) + + if (childExpr.isDefined) { + val builder = ExprOuterClass.Minute.newBuilder() + builder.setChild(childExpr.get) + + // MinutesOfTime is a RuntimeReplaceable expression and doesn't have timeZoneId property. + builder.setTimezone("UTC") + + Some( + ExprOuterClass.Expr + .newBuilder() + .setMinute(builder) + .build()) + } else { + withInfo(expr, expr.children.head) + None + } + } +} + object CometSecond extends CometExpressionSerde[Second] { override def convert( expr: Second, diff --git a/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala b/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala index 2999d8bfe5..7c7e3935c9 100644 --- a/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala +++ b/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala @@ -618,6 +618,21 @@ class CometExpressionSuite extends CometTestBase with AdaptiveSparkPlanHelper { } } + test("MinutesOfTime expression support") { + Seq(true, false).foreach { dictionaryEnabled => + withTempDir { dir => + val path = new Path(dir.toURI.toString, "part-r-0.parquet") + makeRawTimeParquetFile(path, dictionaryEnabled = dictionaryEnabled, 10000) + readParquetFile(path.toString) { df => + // Test that MinutesOfTime (via minute() function) works correctly + val query = df.select(expr("minute(_1)")) + + checkSparkAnswerAndOperator(query) + } + } + } + } + test("hour on int96 timestamp column") { import testImplicits._ From 34e040a6950c46b2b83fe091237ef72572ca4fe3 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sat, 28 Feb 2026 20:44:05 +0800 Subject: [PATCH 4/7] Support MinutesOfTime expression via version-specific shims --- .../apache/comet/serde/QueryPlanSerde.scala | 1 - .../org/apache/comet/serde/datetime.scala | 27 ------------------- .../org/apache/comet/serde/strings.scala | 21 +++++++++++++++ .../apache/comet/shims/CometExprShim.scala | 3 ++- .../apache/comet/shims/CometExprShim.scala | 2 +- .../apache/comet/shims/CometExprShim.scala | 2 +- .../sql-tests/expressions/datetime/minute.sql | 7 +++++ .../apache/comet/CometExpressionSuite.scala | 4 ++- 8 files changed, 35 insertions(+), 32 deletions(-) diff --git a/spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala b/spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala index 63063f0bb1..9d13ccd9ed 100644 --- a/spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala +++ b/spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala @@ -201,7 +201,6 @@ object QueryPlanSerde extends Logging with CometExprShim { classOf[Hour] -> CometHour, classOf[MakeDate] -> CometMakeDate, classOf[Minute] -> CometMinute, - classOf[MinutesOfTime] -> CometMinutesOfTime, classOf[NextDay] -> CometNextDay, classOf[Second] -> CometSecond, classOf[TruncDate] -> CometTruncDate, diff --git a/spark/src/main/scala/org/apache/comet/serde/datetime.scala b/spark/src/main/scala/org/apache/comet/serde/datetime.scala index bf56f1ac7b..d36b6a3b40 100644 --- a/spark/src/main/scala/org/apache/comet/serde/datetime.scala +++ b/spark/src/main/scala/org/apache/comet/serde/datetime.scala @@ -21,7 +21,6 @@ package org.apache.comet.serde import java.util.Locale -import org.apache.spark.sql.catalyst.expressions.{Attribute, DateAdd, DateDiff, DateFormatClass, DateSub, DayOfMonth, DayOfWeek, DayOfYear, GetDateField, Hour, LastDay, Literal, Minute, MinutesOfTime, Month, Quarter, Second, TruncDate, TruncTimestamp, UnixDate, UnixTimestamp, WeekDay, WeekOfYear, Year} import org.apache.spark.sql.catalyst.expressions.{Attribute, DateAdd, DateDiff, DateFormatClass, DateSub, DayOfMonth, DayOfWeek, DayOfYear, GetDateField, Hour, LastDay, Literal, MakeDate, Minute, Month, NextDay, Quarter, Second, TruncDate, TruncTimestamp, UnixDate, UnixTimestamp, WeekDay, WeekOfYear, Year} import org.apache.spark.sql.types.{DateType, IntegerType, StringType, TimestampType} import org.apache.spark.unsafe.types.UTF8String @@ -229,32 +228,6 @@ object CometMinute extends CometExpressionSerde[Minute] { } } -object CometMinutesOfTime extends CometExpressionSerde[MinutesOfTime] { - override def convert( - expr: MinutesOfTime, - inputs: Seq[Attribute], - binding: Boolean): Option[ExprOuterClass.Expr] = { - val childExpr = exprToProtoInternal(expr.children.head, inputs, binding) - - if (childExpr.isDefined) { - val builder = ExprOuterClass.Minute.newBuilder() - builder.setChild(childExpr.get) - - // MinutesOfTime is a RuntimeReplaceable expression and doesn't have timeZoneId property. - builder.setTimezone("UTC") - - Some( - ExprOuterClass.Expr - .newBuilder() - .setMinute(builder) - .build()) - } else { - withInfo(expr, expr.children.head) - None - } - } -} - object CometSecond extends CometExpressionSerde[Second] { override def convert( expr: Second, diff --git a/spark/src/main/scala/org/apache/comet/serde/strings.scala b/spark/src/main/scala/org/apache/comet/serde/strings.scala index 64ba644048..03b35a8d85 100644 --- a/spark/src/main/scala/org/apache/comet/serde/strings.scala +++ b/spark/src/main/scala/org/apache/comet/serde/strings.scala @@ -408,4 +408,25 @@ trait CommonStringExprs { None } } + + def handleMinutesOfTime( + expr: Expression, + inputs: Seq[Attribute], + binding: Boolean): Option[Expr] = { + if (expr.getClass.getSimpleName == "MinutesOfTime" && expr.children.nonEmpty) { + val childExpr = exprToProtoInternal(expr.children.head, inputs, binding) + childExpr.flatMap { ce => + val builder = ExprOuterClass.Minute.newBuilder() + builder.setChild(ce) + builder.setTimezone("UTC") + Some( + ExprOuterClass.Expr + .newBuilder() + .setMinute(builder) + .build()) + } + } else { + None + } + } } diff --git a/spark/src/main/spark-3.4/org/apache/comet/shims/CometExprShim.scala b/spark/src/main/spark-3.4/org/apache/comet/shims/CometExprShim.scala index 600931c346..3e05c16e8e 100644 --- a/spark/src/main/spark-3.4/org/apache/comet/shims/CometExprShim.scala +++ b/spark/src/main/spark-3.4/org/apache/comet/shims/CometExprShim.scala @@ -24,6 +24,7 @@ import org.apache.spark.sql.catalyst.expressions._ import org.apache.comet.expressions.CometEvalMode import org.apache.comet.serde.CommonStringExprs import org.apache.comet.serde.ExprOuterClass.{BinaryOutputStyle, Expr} +import org.apache.comet.serde.QueryPlanSerde /** * `CometExprShim` acts as a shim for parsing expressions from different Spark versions. @@ -43,7 +44,7 @@ trait CometExprShim extends CommonStringExprs { // Right child is the encoding expression. stringDecode(expr, s.charset, s.bin, inputs, binding) - case _ => None + case _ => handleMinutesOfTime(expr, inputs, binding) } } } diff --git a/spark/src/main/spark-3.5/org/apache/comet/shims/CometExprShim.scala b/spark/src/main/spark-3.5/org/apache/comet/shims/CometExprShim.scala index 8e9cb1c07b..ca2da150ce 100644 --- a/spark/src/main/spark-3.5/org/apache/comet/shims/CometExprShim.scala +++ b/spark/src/main/spark-3.5/org/apache/comet/shims/CometExprShim.scala @@ -91,7 +91,7 @@ trait CometExprShim extends CommonStringExprs { // val optExpr = scalarFunctionExprToProto("width_bucket", childExprs: _*) // optExprWithInfo(optExpr, wb, wb.children: _*) - case _ => None + case _ => handleMinutesOfTime(expr, inputs, binding) } } } diff --git a/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala b/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala index 2c5cebd166..7ab26ef729 100644 --- a/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala +++ b/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala @@ -113,7 +113,7 @@ trait CometExprShim extends CommonStringExprs { // val optExpr = scalarFunctionExprToProto("width_bucket", childExprs: _*) // optExprWithInfo(optExpr, wb, wb.children: _*) - case _ => None + case _ => handleMinutesOfTime(expr, inputs, binding) } } } diff --git a/spark/src/test/resources/sql-tests/expressions/datetime/minute.sql b/spark/src/test/resources/sql-tests/expressions/datetime/minute.sql index 0b75084352..e021f75271 100644 --- a/spark/src/test/resources/sql-tests/expressions/datetime/minute.sql +++ b/spark/src/test/resources/sql-tests/expressions/datetime/minute.sql @@ -29,3 +29,10 @@ SELECT minute(ts) FROM test_minute -- literal arguments query ignore(https://github.com/apache/datafusion-comet/issues/3336) SELECT minute(timestamp('2024-01-15 10:00:00')), minute(timestamp('2024-01-15 10:30:00')), minute(timestamp('2024-01-15 10:59:59')) + +-- Note: TIME type is not supported in Spark 3.4-4.0, so we cannot test MinutesOfTime +-- expression directly with TIME literals. However, MinutesOfTime is a RuntimeReplaceable +-- expression that may appear in certain scenarios. Our implementation handles it via +-- version-specific shims (CometExprShim.versionSpecificExprToProtoInternal) which converts +-- it to Minute proto when encountered. The existing timestamp tests above are sufficient +-- to verify that minute() function works correctly. diff --git a/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala b/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala index 15997eaed3..ce4a884984 100644 --- a/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala +++ b/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala @@ -573,12 +573,14 @@ class CometExpressionSuite extends CometTestBase with AdaptiveSparkPlanHelper { } test("MinutesOfTime expression support") { + // This test verifies that minute() function works correctly with timestamp columns. + // If Spark generates MinutesOfTime expression (a RuntimeReplaceable expression), + // it will be handled by the version-specific shim and converted to Minute proto. Seq(true, false).foreach { dictionaryEnabled => withTempDir { dir => val path = new Path(dir.toURI.toString, "part-r-0.parquet") makeRawTimeParquetFile(path, dictionaryEnabled = dictionaryEnabled, 10000) readParquetFile(path.toString) { df => - // Test that MinutesOfTime (via minute() function) works correctly val query = df.select(expr("minute(_1)")) checkSparkAnswerAndOperator(query) From 7cd1cfb27a7f092fdfe6861387a7d0e74a683419 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sun, 1 Mar 2026 12:56:25 +0800 Subject: [PATCH 5/7] remove comment in minute.sql --- .../resources/sql-tests/expressions/datetime/minute.sql | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/spark/src/test/resources/sql-tests/expressions/datetime/minute.sql b/spark/src/test/resources/sql-tests/expressions/datetime/minute.sql index e021f75271..0a1d735969 100644 --- a/spark/src/test/resources/sql-tests/expressions/datetime/minute.sql +++ b/spark/src/test/resources/sql-tests/expressions/datetime/minute.sql @@ -28,11 +28,4 @@ SELECT minute(ts) FROM test_minute -- literal arguments query ignore(https://github.com/apache/datafusion-comet/issues/3336) -SELECT minute(timestamp('2024-01-15 10:00:00')), minute(timestamp('2024-01-15 10:30:00')), minute(timestamp('2024-01-15 10:59:59')) - --- Note: TIME type is not supported in Spark 3.4-4.0, so we cannot test MinutesOfTime --- expression directly with TIME literals. However, MinutesOfTime is a RuntimeReplaceable --- expression that may appear in certain scenarios. Our implementation handles it via --- version-specific shims (CometExprShim.versionSpecificExprToProtoInternal) which converts --- it to Minute proto when encountered. The existing timestamp tests above are sufficient --- to verify that minute() function works correctly. +SELECT minute(timestamp('2024-01-15 10:00:00')), minute(timestamp('2024-01-15 10:30:00')), minute(timestamp('2024-01-15 10:59:59')) \ No newline at end of file From b3962085b37f4c3c576d65d6901e833b5decc7c6 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sun, 1 Mar 2026 13:03:34 +0800 Subject: [PATCH 6/7] style --- .../test/resources/sql-tests/expressions/datetime/minute.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spark/src/test/resources/sql-tests/expressions/datetime/minute.sql b/spark/src/test/resources/sql-tests/expressions/datetime/minute.sql index 0a1d735969..0b75084352 100644 --- a/spark/src/test/resources/sql-tests/expressions/datetime/minute.sql +++ b/spark/src/test/resources/sql-tests/expressions/datetime/minute.sql @@ -28,4 +28,4 @@ SELECT minute(ts) FROM test_minute -- literal arguments query ignore(https://github.com/apache/datafusion-comet/issues/3336) -SELECT minute(timestamp('2024-01-15 10:00:00')), minute(timestamp('2024-01-15 10:30:00')), minute(timestamp('2024-01-15 10:59:59')) \ No newline at end of file +SELECT minute(timestamp('2024-01-15 10:00:00')), minute(timestamp('2024-01-15 10:30:00')), minute(timestamp('2024-01-15 10:59:59')) From ddc579c77af9ed36787cf20425cec10c6f24067d Mon Sep 17 00:00:00 2001 From: Your Name Date: Mon, 2 Mar 2026 13:23:48 +0800 Subject: [PATCH 7/7] modify minute function --- .../org/apache/comet/serde/strings.scala | 49 ++++++++++++++----- .../apache/comet/shims/CometExprShim.scala | 5 +- .../apache/comet/shims/CometExprShim.scala | 5 +- .../apache/comet/shims/CometExprShim.scala | 5 +- 4 files changed, 49 insertions(+), 15 deletions(-) diff --git a/spark/src/main/scala/org/apache/comet/serde/strings.scala b/spark/src/main/scala/org/apache/comet/serde/strings.scala index 03b35a8d85..162b684845 100644 --- a/spark/src/main/scala/org/apache/comet/serde/strings.scala +++ b/spark/src/main/scala/org/apache/comet/serde/strings.scala @@ -409,24 +409,49 @@ trait CommonStringExprs { } } - def handleMinutesOfTime( + def minutesOfTimeToProto( expr: Expression, inputs: Seq[Attribute], binding: Boolean): Option[Expr] = { - if (expr.getClass.getSimpleName == "MinutesOfTime" && expr.children.nonEmpty) { - val childExpr = exprToProtoInternal(expr.children.head, inputs, binding) - childExpr.flatMap { ce => - val builder = ExprOuterClass.Minute.newBuilder() - builder.setChild(ce) - builder.setTimezone("UTC") - Some( + val childOpt = expr.children.headOption.orElse { + withInfo(expr, "MinutesOfTime has no child expression") + None + } + + childOpt.flatMap { child => + val timeZoneId = { + val exprClass = expr.getClass + try { + val timeZoneIdMethod = exprClass.getMethod("timeZoneId") + timeZoneIdMethod.invoke(expr).asInstanceOf[Option[String]] + } catch { + case _: NoSuchMethodException => + try { + val timeZoneIdField = exprClass.getField("timeZoneId") + timeZoneIdField.get(expr).asInstanceOf[Option[String]] + } catch { + case _: NoSuchFieldException | _: SecurityException => None + } + } + } + + exprToProtoInternal(child, inputs, binding) + .map { childExpr => + val builder = ExprOuterClass.Minute.newBuilder() + builder.setChild(childExpr) + + val timeZone = timeZoneId.getOrElse("UTC") + builder.setTimezone(timeZone) + ExprOuterClass.Expr .newBuilder() .setMinute(builder) - .build()) - } - } else { - None + .build() + } + .orElse { + withInfo(expr, child) + None + } } } } diff --git a/spark/src/main/spark-3.4/org/apache/comet/shims/CometExprShim.scala b/spark/src/main/spark-3.4/org/apache/comet/shims/CometExprShim.scala index 3e05c16e8e..e0728b7f46 100644 --- a/spark/src/main/spark-3.4/org/apache/comet/shims/CometExprShim.scala +++ b/spark/src/main/spark-3.4/org/apache/comet/shims/CometExprShim.scala @@ -44,7 +44,10 @@ trait CometExprShim extends CommonStringExprs { // Right child is the encoding expression. stringDecode(expr, s.charset, s.bin, inputs, binding) - case _ => handleMinutesOfTime(expr, inputs, binding) + case _ if expr.getClass.getSimpleName == "MinutesOfTime" => + minutesOfTimeToProto(expr, inputs, binding) + + case _ => None } } } diff --git a/spark/src/main/spark-3.5/org/apache/comet/shims/CometExprShim.scala b/spark/src/main/spark-3.5/org/apache/comet/shims/CometExprShim.scala index ca2da150ce..645c8b4cc3 100644 --- a/spark/src/main/spark-3.5/org/apache/comet/shims/CometExprShim.scala +++ b/spark/src/main/spark-3.5/org/apache/comet/shims/CometExprShim.scala @@ -91,7 +91,10 @@ trait CometExprShim extends CommonStringExprs { // val optExpr = scalarFunctionExprToProto("width_bucket", childExprs: _*) // optExprWithInfo(optExpr, wb, wb.children: _*) - case _ => handleMinutesOfTime(expr, inputs, binding) + case _ if expr.getClass.getSimpleName == "MinutesOfTime" => + minutesOfTimeToProto(expr, inputs, binding) + + case _ => None } } } diff --git a/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala b/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala index 7ab26ef729..7cfbb6abbd 100644 --- a/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala +++ b/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala @@ -113,7 +113,10 @@ trait CometExprShim extends CommonStringExprs { // val optExpr = scalarFunctionExprToProto("width_bucket", childExprs: _*) // optExprWithInfo(optExpr, wb, wb.children: _*) - case _ => handleMinutesOfTime(expr, inputs, binding) + case _ if expr.getClass.getSimpleName == "MinutesOfTime" => + minutesOfTimeToProto(expr, inputs, binding) + + case _ => None } } }