From 945607fa6642d8ff2c1719a74b814c7395904a5f Mon Sep 17 00:00:00 2001 From: Andy Grove Date: Tue, 4 Aug 2026 11:00:55 -0600 Subject: [PATCH] feat: support make_interval via codegen dispatch MakeInterval has no Comet handler today, so any query using make_interval falls the whole operator back to Spark. Route it through the JVM codegen dispatcher, which runs Spark's own generated code inside the native Comet pipeline and so matches Spark bit-for-bit across all supported versions. make_interval produces CalendarIntervalType, which Comet's columnar layer gained support for in #4898; that is what makes carrying the dispatcher's output possible now. Adds make_interval.sql and make_interval_ansi.sql fixtures, the latter covering the ANSI overflow path where MakeInterval.failOnError is true and overflow raises ARITHMETIC_OVERFLOW instead of returning NULL. --- docs/source/user-guide/latest/expressions.md | 2 +- .../apache/comet/serde/QueryPlanSerde.scala | 1 + .../org/apache/comet/serde/datetime.scala | 4 +- .../expressions/datetime/make_interval.sql | 83 +++++++++++++++++++ .../datetime/make_interval_ansi.sql | 49 +++++++++++ 5 files changed, 137 insertions(+), 2 deletions(-) create mode 100644 spark/src/test/resources/sql-tests/expressions/datetime/make_interval.sql create mode 100644 spark/src/test/resources/sql-tests/expressions/datetime/make_interval_ansi.sql diff --git a/docs/source/user-guide/latest/expressions.md b/docs/source/user-guide/latest/expressions.md index 70efdf8559..b6e375c8d1 100644 --- a/docs/source/user-guide/latest/expressions.md +++ b/docs/source/user-guide/latest/expressions.md @@ -277,7 +277,7 @@ The type-name conversion functions (`bigint`, `binary`, `boolean`, `date`, `deci | `localtimestamp` | ✅ | — | | | `make_date` | ✅ | Native | | | `make_dt_interval` | ✅ | Codegen dispatch | | -| `make_interval` | 🔜 | — | Produces legacy CalendarInterval; tracked by [#5061](https://github.com/apache/datafusion-comet/issues/5061) | +| `make_interval` | ✅ | Codegen dispatch | | | `make_time` | 🔜 | — | Spark 4.1 TIME type; tracked by [#4288](https://github.com/apache/datafusion-comet/issues/4288) | | `make_timestamp` | ✅ | Hybrid | | | `make_timestamp_ltz` | ✅ | — | 2-arg TIME form falls back | 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 9e867aad2d..afb9b4d255 100644 --- a/spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala +++ b/spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala @@ -303,6 +303,7 @@ object QueryPlanSerde extends Logging with CometExprShim with CometTypeShim { classOf[MakeTimestamp] -> CometMakeTimestamp, classOf[MakeYMInterval] -> CometMakeYMInterval, classOf[MakeDTInterval] -> CometMakeDTInterval, + classOf[MakeInterval] -> CometMakeInterval, classOf[MultiplyDTInterval] -> CometMultiplyDTInterval, classOf[MicrosToTimestamp] -> CometMicrosToTimestamp, classOf[MillisToTimestamp] -> CometMillisToTimestamp, 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 a2600bf688..a3f393f745 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.{AddMonths, Attribute, ConvertTimezone, DateAdd, DateDiff, DateFormatClass, DateFromUnixDate, DateSub, DayOfMonth, DayOfWeek, DayOfYear, Days, Expression, FromUTCTimestamp, GetDateField, GetTimestamp, Hour, Hours, LastDay, Literal, MakeDate, MakeDTInterval, MakeTimestamp, MakeYMInterval, MicrosToTimestamp, MillisToTimestamp, Minute, Month, MonthsBetween, MultiplyDTInterval, NextDay, PreciseTimestampConversion, Quarter, Second, SecondsToTimestamp, ToUnixTimestamp, ToUTCTimestamp, TruncDate, TruncTimestamp, UnixDate, UnixMicros, UnixMillis, UnixSeconds, UnixTimestamp, WeekDay, WeekOfYear, Year} +import org.apache.spark.sql.catalyst.expressions.{AddMonths, Attribute, ConvertTimezone, DateAdd, DateDiff, DateFormatClass, DateFromUnixDate, DateSub, DayOfMonth, DayOfWeek, DayOfYear, Days, Expression, FromUTCTimestamp, GetDateField, GetTimestamp, Hour, Hours, LastDay, Literal, MakeDate, MakeDTInterval, MakeInterval, MakeTimestamp, MakeYMInterval, MicrosToTimestamp, MillisToTimestamp, Minute, Month, MonthsBetween, MultiplyDTInterval, NextDay, PreciseTimestampConversion, Quarter, Second, SecondsToTimestamp, ToUnixTimestamp, ToUTCTimestamp, TruncDate, TruncTimestamp, UnixDate, UnixMicros, UnixMillis, UnixSeconds, UnixTimestamp, WeekDay, WeekOfYear, Year} import org.apache.spark.sql.internal.SQLConf import org.apache.spark.sql.types.{DataType, DateType, DoubleType, FloatType, IntegerType, LongType, StringType, TimestampNTZType, TimestampType} import org.apache.spark.unsafe.types.UTF8String @@ -968,6 +968,8 @@ object CometMakeYMInterval extends CometCodegenDispatch[MakeYMInterval] object CometMakeDTInterval extends CometCodegenDispatch[MakeDTInterval] +object CometMakeInterval extends CometCodegenDispatch[MakeInterval] + object CometMultiplyDTInterval extends CometCodegenDispatch[MultiplyDTInterval] /** diff --git a/spark/src/test/resources/sql-tests/expressions/datetime/make_interval.sql b/spark/src/test/resources/sql-tests/expressions/datetime/make_interval.sql new file mode 100644 index 0000000000..9c199a6ef4 --- /dev/null +++ b/spark/src/test/resources/sql-tests/expressions/datetime/make_interval.sql @@ -0,0 +1,83 @@ +-- 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. + +-- make_interval runs through the codegen dispatcher and produces CalendarIntervalType. +-- The suite disables ANSI mode, so MakeInterval.failOnError is false here and overflow +-- yields NULL. See make_interval_ansi.sql for the throwing path. +-- Config: spark.comet.exec.scalaUDF.codegen.enabled=true + +statement +CREATE TABLE test_make_interval(y int, mo int, w int, d int, h int, mi int, s decimal(18,6)) USING parquet + +statement +INSERT INTO test_make_interval VALUES + (1, 2, 3, 4, 5, 6, 7.008009), + (30, 25, 0, -100, 40, 80, 299.889987), + (0, -1, 0, 1, 0, 0, -1.000000), + (-1, -2, -3, -4, -5, -6, -7.500000), + (0, 0, 0, 0, 0, 0, 0.000000), + (NULL, 2, 3, 4, 5, 6, 7.008009), + (1, 2, 3, 4, 5, 6, NULL) + +-- all seven arguments as columns +query +SELECT make_interval(y, mo, w, d, h, mi, s) FROM test_make_interval + +-- the shorter arities filled in by MakeInterval's auxiliary constructors +query +SELECT + make_interval(y), + make_interval(y, mo), + make_interval(y, mo, w), + make_interval(y, mo, w, d), + make_interval(y, mo, w, d, h), + make_interval(y, mo, w, d, h, mi) +FROM test_make_interval + +-- mixed literal and column arguments +query +SELECT + make_interval(1, mo, 3, d, 5, mi, 7.008009), + make_interval(y, 2, w, 4, h, 6, s) +FROM test_make_interval + +-- literal arguments (constant folding is disabled by the test suite) +query +SELECT + make_interval(1, 2, 3, 4, 5, 6, 7.008009), + make_interval(0, 0, 0, 0, 0, 0, 0), + make_interval(-1, -2, -3, -4, -5, -6, -7.5), + make_interval(0, 13, 0, 0, 25, 61, 61.5) + +-- NULL arguments propagate. These have to come from nullable columns rather than NULL +-- literals: MakeInterval is NullIntolerant, so NullPropagation rewrites any call with a +-- literal NULL argument to a null interval literal and the expression never reaches Comet. +-- Such a literal currently fails natively rather than falling back, tracked by #5058. +query +SELECT + make_interval(y, 2, 3, 4, 5, 6, 7.008009), + make_interval(1, 2, 3, 4, 5, 6, s), + make_interval(y) +FROM test_make_interval + +-- years * 12 exceeds the int range. Outside ANSI mode MakeInterval swallows the +-- ArithmeticException and returns NULL. +query +SELECT + make_interval(200000000, 0, 0, 0, 0, 0, 0), + make_interval(y + 200000000, mo, w, d, h, mi, s) +FROM test_make_interval diff --git a/spark/src/test/resources/sql-tests/expressions/datetime/make_interval_ansi.sql b/spark/src/test/resources/sql-tests/expressions/datetime/make_interval_ansi.sql new file mode 100644 index 0000000000..f4b91b0604 --- /dev/null +++ b/spark/src/test/resources/sql-tests/expressions/datetime/make_interval_ansi.sql @@ -0,0 +1,49 @@ +-- 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. + +-- MakeInterval.failOnError defaults to SQLConf.get.ansiEnabled, so in ANSI mode overflow +-- raises ARITHMETIC_OVERFLOW instead of returning NULL. The exception has to cross out of +-- the generated kernel and surface as the same Spark error. +-- See make_interval.sql for the non-ANSI coverage. +-- Config: spark.sql.ansi.enabled=true +-- Config: spark.comet.exec.scalaUDF.codegen.enabled=true + +statement +CREATE TABLE test_make_interval_ansi(y int, mo int, w int, d int, h int, mi int, s decimal(18,6)) USING parquet + +statement +INSERT INTO test_make_interval_ansi VALUES + (1, 2, 3, 4, 5, 6, 7.008009), + (-1, -2, -3, -4, -5, -6, -7.500000), + (NULL, 2, 3, 4, 5, 6, 7.008009) + +-- valid inputs still evaluate normally under ANSI, and act as the sentinel proving the +-- expression is not silently falling back to Spark +query +SELECT make_interval(y, mo, w, d, h, mi, s) FROM test_make_interval_ansi + +query +SELECT make_interval(1, 2, 3, 4, 5, 6, 7.008009) + +-- years * 12 overflows the int range. Spark 3.5 renders ARITHMETIC_OVERFLOW without the +-- condition name in the message, so match on the shared "overflow" text instead. +query expect_error(overflow) +SELECT make_interval(200000000, 0, 0, 0, 0, 0, 0) + +-- weeks * 7 overflows the int range +query expect_error(overflow) +SELECT make_interval(0, 0, 400000000, 0, 0, 0, 0)