From 7aba4997d08f6a3df5c5692f83fafa2da9392e5e Mon Sep 17 00:00:00 2001 From: Bolin Lin Date: Thu, 16 Jul 2026 15:25:54 -0400 Subject: [PATCH 1/2] feat: support sorting TimeType values --- .../apache/comet/serde/QueryPlanSerde.scala | 2 + .../expressions/datetime/make_time_sort.sql | 47 +++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 spark/src/test/resources/sql-tests/expressions/datetime/make_time_sort.sql 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 5eee0c5cad..38e770d7d0 100644 --- a/spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala +++ b/spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala @@ -1109,6 +1109,8 @@ object QueryPlanSerde extends Logging with CometExprShim with CometTypeShim { _: DoubleType | _: DecimalType | _: DateType | _: TimestampType | _: TimestampNTZType | _: BooleanType | _: BinaryType | _: StringType => true + case dt if isTimeType(dt) => + true case _ => false } diff --git a/spark/src/test/resources/sql-tests/expressions/datetime/make_time_sort.sql b/spark/src/test/resources/sql-tests/expressions/datetime/make_time_sort.sql new file mode 100644 index 0000000000..f1a9badfb8 --- /dev/null +++ b/spark/src/test/resources/sql-tests/expressions/datetime/make_time_sort.sql @@ -0,0 +1,47 @@ +-- Licensed to the Apache Software Foundation (ASF) under one +-- or more contributor license agreements. See the NOTICE file +-- distributed with this work for additional information +-- regarding copyright ownership. The ASF licenses this file +-- to you under the Apache License, Version 2.0 (the +-- "License"); you may not use this file except in compliance +-- with the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, +-- software distributed under the License is distributed on an +-- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +-- KIND, either express or implied. See the License for the +-- specific language governing permissions and limitations +-- under the License. + +-- MinSparkVersion: 4.1 +-- Config: spark.sql.timeType.enabled=true + +statement +CREATE TABLE test_make_time_sort(id int, hours int, minutes int, secs decimal(16,6)) USING parquet + +statement +INSERT INTO test_make_time_sort VALUES + (1, 12, 30, 45.123456), + (2, 0, 0, 0.000000), + (3, 23, 59, 59.999999), + (4, 1, 2, 3.500000), + (5, 0, 0, 0.000001), + (6, NULL, NULL, NULL) + +query +SELECT id, t +FROM ( + SELECT id, make_time(hours, minutes, secs) AS t + FROM test_make_time_sort +) +SORT BY t ASC NULLS FIRST + +query +SELECT id, t +FROM ( + SELECT id, make_time(hours, minutes, secs) AS t + FROM test_make_time_sort +) +SORT BY t DESC NULLS LAST From 968c30460f05fc649bd99e7dcb6d503540273aad Mon Sep 17 00:00:00 2001 From: Bolin Lin Date: Fri, 17 Jul 2026 16:34:11 -0400 Subject: [PATCH 2/2] feat: support TimeType min and max aggregates --- .../org/apache/comet/serde/aggregates.scala | 5 +- .../datetime/make_time_min_max.sql | 55 +++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 spark/src/test/resources/sql-tests/expressions/datetime/make_time_min_max.sql diff --git a/spark/src/main/scala/org/apache/comet/serde/aggregates.scala b/spark/src/main/scala/org/apache/comet/serde/aggregates.scala index 1fb676366c..d757e0c1a1 100644 --- a/spark/src/main/scala/org/apache/comet/serde/aggregates.scala +++ b/spark/src/main/scala/org/apache/comet/serde/aggregates.scala @@ -31,7 +31,7 @@ import org.apache.comet.CometConf.COMET_EXEC_STRICT_FLOATING_POINT import org.apache.comet.CometSparkSessionExtensions.{isSpark41Plus, withFallbackReason} import org.apache.comet.expressions.CometEvalMode import org.apache.comet.serde.QueryPlanSerde.{evalModeToProto, exprToProto, serializeDataType} -import org.apache.comet.shims.CometEvalModeUtil +import org.apache.comet.shims.{CometEvalModeUtil, CometTypeShim} object CometMin extends CometAggregateExpressionSerde[Min] { @@ -897,7 +897,7 @@ object CometCollectSet extends CometAggregateExpressionSerde[CollectSet] { } } -object AggSerde { +object AggSerde extends CometTypeShim { import org.apache.spark.sql.types._ def minMaxDataTypeSupported(dt: DataType): Boolean = { @@ -907,6 +907,7 @@ object AggSerde { case FloatType | DoubleType => true case _: DecimalType => true case DateType | TimestampType => true + case dt if isTimeType(dt) => true case _ => false } } diff --git a/spark/src/test/resources/sql-tests/expressions/datetime/make_time_min_max.sql b/spark/src/test/resources/sql-tests/expressions/datetime/make_time_min_max.sql new file mode 100644 index 0000000000..c6ee6f553f --- /dev/null +++ b/spark/src/test/resources/sql-tests/expressions/datetime/make_time_min_max.sql @@ -0,0 +1,55 @@ +-- Licensed to the Apache Software Foundation (ASF) under one +-- or more contributor license agreements. See the NOTICE file +-- distributed with this work for additional information +-- regarding copyright ownership. The ASF licenses this file +-- to you under the Apache License, Version 2.0 (the +-- "License"); you may not use this file except in compliance +-- with the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, +-- software distributed under the License is distributed on an +-- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +-- KIND, either express or implied. See the License for the +-- specific language governing permissions and limitations +-- under the License. + +-- MinSparkVersion: 4.1 +-- Config: spark.sql.timeType.enabled=true + +statement +CREATE TABLE test_make_time_min_max(grp string, hours int, minutes int, secs decimal(16,6)) USING parquet + +statement +INSERT INTO test_make_time_min_max VALUES + ('a', 12, 30, 45.123456), + ('a', 0, 0, 0.000000), + ('a', NULL, NULL, NULL), + ('b', 23, 59, 59.999999), + ('b', 1, 2, 3.500000), + ('c', NULL, 0, 0.000000), + ('c', 12, NULL, 0.000000), + ('c', 12, 30, NULL) + +query +SELECT + min(make_time(hours, minutes, secs)) AS min_time, + max(make_time(hours, minutes, secs)) AS max_time +FROM test_make_time_min_max + +query +SELECT + grp, + min(make_time(hours, minutes, secs)) AS min_time, + max(make_time(hours, minutes, secs)) AS max_time +FROM test_make_time_min_max +GROUP BY grp +ORDER BY grp + +query +SELECT + min(make_time(hours, minutes, secs)) AS min_time, + max(make_time(hours, minutes, secs)) AS max_time +FROM test_make_time_min_max +WHERE grp = 'c'