From e269eb24170c162be82eda7fcdaad64b7612710f Mon Sep 17 00:00:00 2001 From: peterxcli Date: Sun, 26 Jul 2026 02:40:02 +0800 Subject: [PATCH 1/2] Add native make_interval support --- docs/source/user-guide/latest/expressions.md | 2 +- native/Cargo.lock | 1 + native/core/src/execution/jni_api.rs | 2 + native/spark-expr/Cargo.toml | 1 + native/spark-expr/src/comet_scalar_funcs.rs | 7 +- .../src/datetime_funcs/make_interval.rs | 82 +++++++++++++++++++ native/spark-expr/src/datetime_funcs/mod.rs | 2 + native/spark-expr/src/lib.rs | 5 +- .../apache/comet/serde/QueryPlanSerde.scala | 10 ++- .../org/apache/comet/serde/datetime.scala | 13 ++- .../expressions/datetime/make_interval.sql | 46 +++++++++++ .../datetime/make_interval_ansi.sql | 35 ++++++++ 12 files changed, 199 insertions(+), 7 deletions(-) create mode 100644 native/spark-expr/src/datetime_funcs/make_interval.rs 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 c292700dbc..f987642bee 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 [#4540](https://github.com/apache/datafusion-comet/issues/4540) | +| `make_interval` | ✅ | Native | | | `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/native/Cargo.lock b/native/Cargo.lock index 8194fdd70a..4dfeb9c074 100644 --- a/native/Cargo.lock +++ b/native/Cargo.lock @@ -2065,6 +2065,7 @@ dependencies = [ "datafusion", "datafusion-comet-common", "datafusion-comet-jni-bridge", + "datafusion-spark", "futures", "jni 0.22.4", "num", diff --git a/native/core/src/execution/jni_api.rs b/native/core/src/execution/jni_api.rs index 9a82eb6746..da0d880dc1 100644 --- a/native/core/src/execution/jni_api.rs +++ b/native/core/src/execution/jni_api.rs @@ -43,6 +43,7 @@ use datafusion::{ }; use datafusion_comet_proto::spark_operator::Operator; use datafusion_comet_spark_expr::url_funcs::{CometParseUrl, CometTryParseUrl}; +use datafusion_comet_spark_expr::SparkMakeInterval; use datafusion_spark::function::array::array_contains::SparkArrayContains; use datafusion_spark::function::array::repeat::SparkArrayRepeat; use datafusion_spark::function::bitwise::bit_count::SparkBitCount; @@ -618,6 +619,7 @@ fn register_datafusion_spark_function(session_ctx: &SessionContext) { session_ctx.register_udf(ScalarUDF::new_from_impl(SparkDateSub::default())); session_ctx.register_udf(ScalarUDF::new_from_impl(SparkFromUtcTimestamp::default())); session_ctx.register_udf(ScalarUDF::new_from_impl(SparkLastDay::default())); + session_ctx.register_udf(ScalarUDF::new_from_impl(SparkMakeInterval::default())); session_ctx.register_udf(ScalarUDF::new_from_impl(SparkToUtcTimestamp::default())); session_ctx.register_udf(ScalarUDF::new_from_impl(SparkSha1::default())); session_ctx.register_udf(ScalarUDF::new_from_impl(SparkConcat::default())); diff --git a/native/spark-expr/Cargo.toml b/native/spark-expr/Cargo.toml index c05ae89793..70eedf810c 100644 --- a/native/spark-expr/Cargo.toml +++ b/native/spark-expr/Cargo.toml @@ -30,6 +30,7 @@ edition = { workspace = true } arrow = { workspace = true } chrono = { workspace = true } datafusion = { workspace = true } +datafusion-spark = { workspace = true } chrono-tz = { workspace = true } num = { workspace = true } regex = { workspace = true } diff --git a/native/spark-expr/src/comet_scalar_funcs.rs b/native/spark-expr/src/comet_scalar_funcs.rs index 8e913dabab..ed85c8cee0 100644 --- a/native/spark-expr/src/comet_scalar_funcs.rs +++ b/native/spark-expr/src/comet_scalar_funcs.rs @@ -27,8 +27,8 @@ use crate::{ spark_isnan, spark_lpad, spark_make_decimal, spark_month_name, spark_read_side_padding, spark_round, spark_rpad, spark_to_time, spark_unhex, spark_unscaled_value, EvalMode, SparkArrayPositionFunc, SparkArraySlice, SparkArraysOverlap, SparkContains, SparkDateDiff, - SparkDateFromUnixDate, SparkDateTrunc, SparkFlatten, SparkMakeDate, SparkMakeTime, - SparkNextDay, SparkSecondsToTimestamp, SparkSizeFunc, + SparkDateFromUnixDate, SparkDateTrunc, SparkFlatten, SparkMakeDate, SparkMakeInterval, + SparkMakeTime, SparkNextDay, SparkSecondsToTimestamp, SparkSizeFunc, }; use arrow::datatypes::DataType; use datafusion::common::{DataFusionError, Result as DataFusionResult}; @@ -230,6 +230,9 @@ pub fn create_comet_physical_fun_with_eval_mode( "make_date" => Ok(Arc::new(ScalarUDF::new_from_impl(SparkMakeDate::new( fail_on_error, )))), + "make_interval" => Ok(Arc::new(ScalarUDF::new_from_impl(SparkMakeInterval::new( + fail_on_error, + )))), "next_day" => Ok(Arc::new(ScalarUDF::new_from_impl(SparkNextDay::new( fail_on_error, )))), diff --git a/native/spark-expr/src/datetime_funcs/make_interval.rs b/native/spark-expr/src/datetime_funcs/make_interval.rs new file mode 100644 index 0000000000..ad9996959e --- /dev/null +++ b/native/spark-expr/src/datetime_funcs/make_interval.rs @@ -0,0 +1,82 @@ +// 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. + +use crate::arithmetic_overflow_error; +use arrow::array::Array; +use arrow::datatypes::DataType; +use datafusion::common::Result; +use datafusion::logical_expr::{ColumnarValue, ScalarFunctionArgs, ScalarUDFImpl, Signature}; +use datafusion_spark::function::datetime::make_interval::SparkMakeInterval as DataFusionMakeInterval; + +#[derive(Debug, PartialEq, Eq, Hash)] +pub struct SparkMakeInterval { + inner: DataFusionMakeInterval, + fail_on_error: bool, +} + +impl SparkMakeInterval { + pub fn new(fail_on_error: bool) -> Self { + Self { + inner: DataFusionMakeInterval::new(), + fail_on_error, + } + } +} + +impl Default for SparkMakeInterval { + fn default() -> Self { + Self::new(false) + } +} + +impl ScalarUDFImpl for SparkMakeInterval { + fn name(&self) -> &str { + self.inner.name() + } + + fn signature(&self) -> &Signature { + self.inner.signature() + } + + fn return_type(&self, arg_types: &[DataType]) -> Result { + self.inner.return_type(arg_types) + } + + fn invoke_with_args(&self, args: ScalarFunctionArgs) -> Result { + let inputs = if self.fail_on_error && !args.args.is_empty() { + Some(ColumnarValue::values_to_arrays(&args.args)?) + } else { + None + }; + let result = self.inner.invoke_with_args(args)?; + + if let Some(inputs) = inputs { + let overflow = match &result { + ColumnarValue::Array(values) => (0..values.len()) + .any(|i| values.is_null(i) && inputs.iter().all(|a| !a.is_null(i))), + ColumnarValue::Scalar(value) => { + value.is_null() && inputs.iter().all(|a| !a.is_null(0)) + } + }; + if overflow { + return Err(arithmetic_overflow_error("interval").into()); + } + } + + Ok(result) + } +} diff --git a/native/spark-expr/src/datetime_funcs/mod.rs b/native/spark-expr/src/datetime_funcs/mod.rs index 37c5fa5dd2..05530f29c2 100644 --- a/native/spark-expr/src/datetime_funcs/mod.rs +++ b/native/spark-expr/src/datetime_funcs/mod.rs @@ -22,6 +22,7 @@ mod day_month_name; mod extract_date_part; mod hours; mod make_date; +mod make_interval; mod make_time; mod next_day; mod seconds_to_timestamp; @@ -38,6 +39,7 @@ pub use extract_date_part::SparkMinute; pub use extract_date_part::SparkSecond; pub use hours::SparkHoursTransform; pub use make_date::SparkMakeDate; +pub use make_interval::SparkMakeInterval; pub use make_time::SparkMakeTime; pub use next_day::SparkNextDay; pub use seconds_to_timestamp::SparkSecondsToTimestamp; diff --git a/native/spark-expr/src/lib.rs b/native/spark-expr/src/lib.rs index 40422cde07..19a0775291 100644 --- a/native/spark-expr/src/lib.rs +++ b/native/spark-expr/src/lib.rs @@ -77,8 +77,9 @@ pub use comet_scalar_funcs::{ pub use csv_funcs::*; pub use datetime_funcs::{ spark_day_name, spark_month_name, spark_to_time, SparkDateDiff, SparkDateFromUnixDate, - SparkDateTrunc, SparkHour, SparkHoursTransform, SparkMakeDate, SparkMakeTime, SparkMinute, - SparkNextDay, SparkSecond, SparkSecondsToTimestamp, SparkUnixTimestamp, TimestampTruncExpr, + SparkDateTrunc, SparkHour, SparkHoursTransform, SparkMakeDate, SparkMakeInterval, + SparkMakeTime, SparkMinute, SparkNextDay, SparkSecond, SparkSecondsToTimestamp, + SparkUnixTimestamp, TimestampTruncExpr, }; pub use error::{decimal_overflow_error, SparkError, SparkErrorWithContext, SparkResult}; pub use hash_funcs::*; 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 6f8306862a..961431838e 100644 --- a/spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala +++ b/spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala @@ -301,6 +301,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, @@ -1068,9 +1069,16 @@ object QueryPlanSerde extends Logging with CometExprShim with CometTypeShim { } def scalarFunctionExprToProto(funcName: String, args: Option[Expr]*): Option[Expr] = { + scalarFunctionExprToProto(funcName, false, args: _*) + } + + def scalarFunctionExprToProto( + funcName: String, + failOnError: Boolean, + args: Option[Expr]*): Option[Expr] = { val builder = ExprOuterClass.ScalarFunc.newBuilder() builder.setFunc(funcName) - builder.setFailOnError(false) + builder.setFailOnError(failOnError) scalarFunctionExprToProto0(builder, args: _*) } 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 d968c00cd1..a32627c83a 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 @@ -954,6 +954,17 @@ object CometMakeYMInterval extends CometCodegenDispatch[MakeYMInterval] object CometMakeDTInterval extends CometCodegenDispatch[MakeDTInterval] +object CometMakeInterval extends CometExpressionSerde[MakeInterval] { + override def convert( + expr: MakeInterval, + inputs: Seq[Attribute], + binding: Boolean): Option[Expr] = { + val childExprs = expr.children.map(exprToProtoInternal(_, inputs, binding)) + val optExpr = scalarFunctionExprToProto("make_interval", expr.failOnError, childExprs: _*) + optExprWithFallbackReason(optExpr, expr, expr.children: _*) + } +} + 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..924bbd5fa6 --- /dev/null +++ b/spark/src/test/resources/sql-tests/expressions/datetime/make_interval.sql @@ -0,0 +1,46 @@ +-- 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. + +-- Config: spark.comet.exec.scalaUDF.codegen.enabled=false + +statement +CREATE TABLE test_make_interval( + years int, + months int, + weeks int, + days int, + hours int, + mins int, + secs decimal(38, 6)) USING parquet + +statement +INSERT INTO test_make_interval VALUES + (1, 2, 3, 4, 5, 6, 7.123456), + (0, 1, 0, 1, 0, 0, 100.000001), + (-1, -2, -1, -1, -1, -1, -1.500000), + (NULL, 1, 2, 3, 4, 5, 6.000000) + +query +SELECT make_interval(years, months, weeks, days, hours, mins, secs) +FROM test_make_interval +ORDER BY years + +query +SELECT make_interval(1, 2), make_interval(3), make_interval() + +query +SELECT make_interval(2147483647) 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..1e521cff68 --- /dev/null +++ b/spark/src/test/resources/sql-tests/expressions/datetime/make_interval_ansi.sql @@ -0,0 +1,35 @@ +-- 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. + +-- Native ANSI execution must preserve Spark's overflow exception. +-- Config: spark.sql.ansi.enabled=true +-- Config: spark.comet.exec.scalaUDF.codegen.enabled=false + +statement +CREATE TABLE test_make_interval_ansi(years int) USING parquet + +statement +INSERT INTO test_make_interval_ansi VALUES (NULL) + +query +SELECT make_interval(1, 2, 3, 4, 5, 6, 7.123456) + +query +SELECT make_interval(years) FROM test_make_interval_ansi + +query expect_error(overflow) +SELECT make_interval(2147483647) From 69eb4aa061117b6b7b1d7c7df451d5b2693bd4ed Mon Sep 17 00:00:00 2001 From: peterxcli Date: Wed, 29 Jul 2026 23:21:37 +0800 Subject: [PATCH 2/2] review --- docs/source/user-guide/latest/expressions.md | 2 +- .../src/datetime_funcs/make_interval.rs | 23 +++++++++++----- .../org/apache/comet/serde/datetime.scala | 26 +++++++++++++++---- .../expressions/datetime/make_interval.sql | 24 ++++++++++++++--- .../datetime/make_interval_ansi.sql | 5 +++- 5 files changed, 63 insertions(+), 17 deletions(-) diff --git a/docs/source/user-guide/latest/expressions.md b/docs/source/user-guide/latest/expressions.md index 28f7a9a7c4..43b69442e1 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` | ✅ | Native | | +| `make_interval` | ✅ | Native | Falls back by default; opt-in via allowIncompatible ([details](compatibility/expressions/datetime.md)) | | `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/native/spark-expr/src/datetime_funcs/make_interval.rs b/native/spark-expr/src/datetime_funcs/make_interval.rs index ad9996959e..d6105ba87f 100644 --- a/native/spark-expr/src/datetime_funcs/make_interval.rs +++ b/native/spark-expr/src/datetime_funcs/make_interval.rs @@ -57,20 +57,29 @@ impl ScalarUDFImpl for SparkMakeInterval { } fn invoke_with_args(&self, args: ScalarFunctionArgs) -> Result { - let inputs = if self.fail_on_error && !args.args.is_empty() { - Some(ColumnarValue::values_to_arrays(&args.args)?) + let inputs = if self.fail_on_error { + Some(args.args.clone()) } else { None }; let result = self.inner.invoke_with_args(args)?; if let Some(inputs) = inputs { + let inputs_are_valid = |i| { + inputs.iter().all(|input| match input { + ColumnarValue::Array(values) => values.is_valid(i), + ColumnarValue::Scalar(value) => !value.is_null(), + }) + }; let overflow = match &result { - ColumnarValue::Array(values) => (0..values.len()) - .any(|i| values.is_null(i) && inputs.iter().all(|a| !a.is_null(i))), - ColumnarValue::Scalar(value) => { - value.is_null() && inputs.iter().all(|a| !a.is_null(0)) - } + ColumnarValue::Array(values) => values.nulls().is_some_and(|nulls| { + nulls.null_count() != 0 + && nulls + .iter() + .enumerate() + .any(|(i, is_valid)| !is_valid && inputs_are_valid(i)) + }), + ColumnarValue::Scalar(value) => value.is_null() && inputs_are_valid(0), }; if overflow { return Err(arithmetic_overflow_error("interval").into()); 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 a32627c83a..570853924c 100644 --- a/spark/src/main/scala/org/apache/comet/serde/datetime.scala +++ b/spark/src/main/scala/org/apache/comet/serde/datetime.scala @@ -21,9 +21,9 @@ 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, 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.catalyst.expressions.{AddMonths, Attribute, Cast, 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.sql.types.{CalendarIntervalType, DataType, DateType, DoubleType, FloatType, IntegerType, LongType, StringType, TimestampNTZType, TimestampType} import org.apache.spark.unsafe.types.UTF8String import org.apache.comet.CometConf @@ -955,13 +955,29 @@ object CometMakeYMInterval extends CometCodegenDispatch[MakeYMInterval] object CometMakeDTInterval extends CometCodegenDispatch[MakeDTInterval] object CometMakeInterval extends CometExpressionSerde[MakeInterval] { + private val incompatReason = + "The native implementation converts seconds to `Float64`, which can lose microsecond" + + " precision, and stores time in nanoseconds, which overflows for large seconds values" + + " that Spark can represent." + + override def getIncompatibleReasons(): Seq[String] = Seq(incompatReason) + + override def getSupportLevel(expr: MakeInterval): SupportLevel = + Incompatible(Some(incompatReason)) + override def convert( expr: MakeInterval, inputs: Seq[Attribute], binding: Boolean): Option[Expr] = { - val childExprs = expr.children.map(exprToProtoInternal(_, inputs, binding)) - val optExpr = scalarFunctionExprToProto("make_interval", expr.failOnError, childExprs: _*) - optExprWithFallbackReason(optExpr, expr, expr.children: _*) + // The explicit return type skips DataFusion's registry coercion, but its kernel needs Float64. + val children = expr.children.updated(6, Cast(expr.children(6), DoubleType)) + val childExprs = children.map(exprToProtoInternal(_, inputs, binding)) + val optExpr = scalarFunctionExprToProtoWithReturnType( + "make_interval", + CalendarIntervalType, + expr.failOnError, + childExprs: _*) + optExprWithFallbackReason(optExpr, expr, children: _*) } } 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 index 924bbd5fa6..5f7bc6be2f 100644 --- a/spark/src/test/resources/sql-tests/expressions/datetime/make_interval.sql +++ b/spark/src/test/resources/sql-tests/expressions/datetime/make_interval.sql @@ -15,7 +15,7 @@ -- specific language governing permissions and limitations -- under the License. --- Config: spark.comet.exec.scalaUDF.codegen.enabled=false +-- Config: spark.comet.expression.MakeInterval.allowIncompatible=true statement CREATE TABLE test_make_interval( @@ -25,14 +25,17 @@ CREATE TABLE test_make_interval( days int, hours int, mins int, - secs decimal(38, 6)) USING parquet + secs decimal(18, 6)) USING parquet statement INSERT INTO test_make_interval VALUES (1, 2, 3, 4, 5, 6, 7.123456), (0, 1, 0, 1, 0, 0, 100.000001), (-1, -2, -1, -1, -1, -1, -1.500000), - (NULL, 1, 2, 3, 4, 5, 6.000000) + (NULL, 1, 2, 3, 4, 5, 6.000000), + (2, NULL, 2, 3, 4, 5, 6.000000), + (3, 1, 2, 3, 4, 5, NULL), + (-2147483648, 0, 0, 0, 0, 0, 0.000000) query SELECT make_interval(years, months, weeks, days, hours, mins, secs) @@ -42,5 +45,20 @@ ORDER BY years query SELECT make_interval(1, 2), make_interval(3), make_interval() +query +SELECT make_interval(0, 1, 0, 1, 0, 0, 100.000001) + query SELECT make_interval(2147483647) + +query ignore(https://github.com/apache/datafusion-comet/issues/5131) +SELECT make_interval(1, 2, 3, 4, 0, 0, 123456789012.123456) + +query +SELECT make_interval(0, 0, 0, 0, 0, 0, 999999999.999999) + +query ignore(https://github.com/apache/datafusion-comet/issues/5131) +SELECT make_interval(0, 0, 0, 0, 0, 0, 999999999.000001) + +query +SELECT make_interval(0, 0, 0, 0, 0, 0, 1234567890123456789) 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 index 1e521cff68..a56dbfc7d1 100644 --- 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 @@ -17,7 +17,7 @@ -- Native ANSI execution must preserve Spark's overflow exception. -- Config: spark.sql.ansi.enabled=true --- Config: spark.comet.exec.scalaUDF.codegen.enabled=false +-- Config: spark.comet.expression.MakeInterval.allowIncompatible=true statement CREATE TABLE test_make_interval_ansi(years int) USING parquet @@ -33,3 +33,6 @@ SELECT make_interval(years) FROM test_make_interval_ansi query expect_error(overflow) SELECT make_interval(2147483647) + +query expect_error(overflow) +SELECT make_interval(0, 0, 2147483647)