Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/source/user-guide/latest/expressions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion spark/src/main/scala/org/apache/comet/serde/datetime.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -968,6 +968,8 @@ object CometMakeYMInterval extends CometCodegenDispatch[MakeYMInterval]

object CometMakeDTInterval extends CometCodegenDispatch[MakeDTInterval]

object CometMakeInterval extends CometCodegenDispatch[MakeInterval]

object CometMultiplyDTInterval extends CometCodegenDispatch[MultiplyDTInterval]

/**
Expand Down

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could also add test cases which is from the review of native make_interval impl: #5039 (comment)

Spark's own sql-tests/inputs/interval.sql exercises exactly this range:

select make_interval(1, 2, 3, 4, 0, 0, 123456789012.123456);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other reference: #5039 (comment)

Spark's own IntervalExpressionsSuite / interval.sql exercise that aren't covered yet:

  • Microsecond-precision seconds like Spark's docstring example make_interval(0, 1, 0, 1, 0, 0, 100.000001) asserted directly (it's currently only exercised via the column path where it can be hard to spot a per-row precision drift).
  • Nulls in components other than years in the column path (currently only the years=NULL row is tested).
  • Large-second cases from Spark's interval.sql: make_interval(1, 2, 3, 4, 0, 0, 123456789012.123456) and make_interval(0, 0, 0, 0, 0, 0, 1234567890123456789). If either is a known divergence (see the nanos-overflow comment on the Rust file), wrapping them in query ignore() would at least pin the behavior for future readers.
  • Int.MinValue for a signed-overflow smoke test on the years column.

Original file line number Diff line number Diff line change
@@ -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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto as `make_interval.sql.
ref: #5039 (comment)

Spark's IntervalExpressionsSuite ANSI mode block covers weeks = Int.MaxValue, and per-row overflow via hours/mins/seconds interactions. Something like:

query expect_error(overflow)
SELECT make_interval(0, 0, 2147483647)

Original file line number Diff line number Diff line change
@@ -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)
Loading