This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-5135-7f104e7e9fab1d25a1604b50ac4ac9141cd12088 in repository https://gitbox.apache.org/repos/asf/datafusion-comet.git
commit 5eed2015bdcd798f45b0f800d73e98b2112d9d72 Author: Peter Lee <[email protected]> AuthorDate: Wed Sep 23 20:24:33 2026 +0000 fix: support CalendarIntervalType hashing (#5135) * fix: support CalendarIntervalType hashing * address CalendarInterval hash review feedback * docs: update CalendarIntervalType support * prettier * docs: document nanoseconds-to-microseconds invariant in interval hash macro Both JVM-to-Arrow producers convert with Math.multiplyExact(micros, 1000L), so nanoseconds is always an exact multiple of 1000 and out-of-range intervals fail loudly at conversion time. Co-Authored-By: Claude Fable 5 <[email protected]> --------- Co-authored-by: Claude Fable 5 <[email protected]> --- docs/source/user-guide/latest/datatypes.md | 12 ++--- native/spark-expr/src/hash_funcs/utils.rs | 56 +++++++++++++++++++++- .../expressions/datetime/calendar_interval.sql | 40 ++++++++++++++++ 3 files changed, 101 insertions(+), 7 deletions(-) diff --git a/docs/source/user-guide/latest/datatypes.md b/docs/source/user-guide/latest/datatypes.md index c2b942961c..06b48e6205 100644 --- a/docs/source/user-guide/latest/datatypes.md +++ b/docs/source/user-guide/latest/datatypes.md @@ -84,14 +84,14 @@ All three interval types are mapped to Arrow and flow through serde, native shuf codegen dispatcher, so interval columns and the interval-producing expressions run natively. Several operators still gate on the type and fall back: Parquet scans of ANSI interval columns, single-column sorts, hash aggregates (`min` / `max` / `sum` / `avg`), `GROUP BY`, window -functions, and hashing a `CalendarInterval`. Remaining work is tracked by +functions. Remaining work is tracked by [#5061](https://github.com/apache/datafusion-comet/issues/5061). -| Type | Status | Notes | -| ----------------------- | ------ | ------------------------------------------------------------------------------------------ | -| `YearMonthIntervalType` | ⚠️ | Parquet scan, single-column sort, aggregate, `GROUP BY`, and window operators fall back. | -| `DayTimeIntervalType` | ⚠️ | Parquet scan, single-column sort, aggregate, `GROUP BY`, and window operators fall back. | -| `CalendarIntervalType` | ⚠️ | As above, plus `hash` / `xxhash64` of a `CalendarInterval` fails rather than falling back. | +| Type | Status | Notes | +| ----------------------- | ------ | ---------------------------------------------------------------------------------------- | +| `YearMonthIntervalType` | ⚠️ | Parquet scan, single-column sort, aggregate, `GROUP BY`, and window operators fall back. | +| `DayTimeIntervalType` | ⚠️ | Parquet scan, single-column sort, aggregate, `GROUP BY`, and window operators fall back. | +| `CalendarIntervalType` | ⚠️ | Parquet scan, single-column sort, aggregate, `GROUP BY`, and window operators fall back. | ## Complex diff --git a/native/spark-expr/src/hash_funcs/utils.rs b/native/spark-expr/src/hash_funcs/utils.rs index db9f95db82..7411b8dbfe 100644 --- a/native/spark-expr/src/hash_funcs/utils.rs +++ b/native/spark-expr/src/hash_funcs/utils.rs @@ -157,6 +157,53 @@ macro_rules! hash_array_primitive_float { }; } +#[macro_export] +macro_rules! hash_array_interval_month_day_nano { + ($column: ident, $hashes: ident, $hash_method: ident) => { + let array = $column + .as_any() + .downcast_ref::<IntervalMonthDayNanoArray>() + .unwrap_or_else(|| { + panic!( + "Failed to downcast column to {}. Actual data type: {:?}.", + stringify!(IntervalMonthDayNanoArray), + $column.data_type() + ) + }); + + // The `nanoseconds / 1_000` below is exact: Spark's `CalendarInterval` is + // microsecond-based, and both JVM-to-Arrow producers + // (`ArrowWriters.CalendarIntervalWriter` and the codegen dispatch kernel) convert + // with `Math.multiplyExact(microseconds, 1000L)`, so the nanoseconds field is + // always an exact multiple of 1000 and out-of-range intervals throw at + // conversion time instead of reaching this hasher. + if array.null_count() == 0 { + // Fast path: no nulls, use direct indexing + for i in 0..$hashes.len() { + let value = array.value(i); + // Match Spark 4.2 generated code, which omits the days field: + // https://github.com/apache/spark/blob/v4.2.0/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/hash.scala#L428-L431 + // SPARK-58236 includes days starting in Spark 4.3; the version + // switch for that is tracked in + // https://github.com/apache/datafusion-comet/issues/5498. + $hashes[i] = + $hash_method((value.nanoseconds / 1_000).to_le_bytes(), $hashes[i]); + $hashes[i] = $hash_method(value.months.to_le_bytes(), $hashes[i]); + } + } else { + // Slow path: check nulls + for i in 0..$hashes.len() { + if !array.is_null(i) { + let value = array.value(i); + $hashes[i] = + $hash_method((value.nanoseconds / 1_000).to_le_bytes(), $hashes[i]); + $hashes[i] = $hash_method(value.months.to_le_bytes(), $hashes[i]); + } + } + } + }; +} + #[macro_export] macro_rules! hash_array_small_decimal { ($array_type:ident, $column: ident, $hashes: ident, $hash_method: ident) => { @@ -789,7 +836,7 @@ macro_rules! hash_list_array { #[macro_export] macro_rules! create_hashes_internal { ($arrays: ident, $hashes_buffer: ident, $hash_method: ident, $create_dictionary_hash_method: ident, $recursive_hash_method: ident) => { - use arrow::datatypes::{DataType, TimeUnit}; + use arrow::datatypes::{DataType, IntervalUnit, TimeUnit}; use arrow::array::{types::*, *}; use datafusion_comet_common::children_with_parent_nulls; @@ -933,6 +980,13 @@ macro_rules! create_hashes_internal { $hash_method ); } + DataType::Interval(IntervalUnit::MonthDayNano) => { + $crate::hash_array_interval_month_day_nano!( + col, + $hashes_buffer, + $hash_method + ); + } DataType::Utf8 => { $crate::hash_array!(StringArray, col, $hashes_buffer, $hash_method); } diff --git a/spark/src/test/resources/sql-tests/expressions/datetime/calendar_interval.sql b/spark/src/test/resources/sql-tests/expressions/datetime/calendar_interval.sql index 6eaa9648f2..2f335f3512 100644 --- a/spark/src/test/resources/sql-tests/expressions/datetime/calendar_interval.sql +++ b/spark/src/test/resources/sql-tests/expressions/datetime/calendar_interval.sql @@ -51,3 +51,43 @@ FROM ( AS t(id, a) DISTRIBUTE BY id ) + +-- Exact column-based repro from #5059. +query +SELECT + hash(make_interval(y, m, 0, d, h, 0, 0)), + xxhash64(make_interval(y, m, 0, d, h, 0, 0)) +FROM VALUES + (1, 2, 1, 2), + (0, 0, 0, 0), + (-1, -2, -1, -2) +AS t(y, m, d, h) + +-- Field-only values, nulls, seed chaining, and recursive array/struct hashing. +query +SELECT + hash(c), + xxhash64(c), + hash(c, 1), + hash(c, 0), + hash(c, n), + xxhash64(c, 1), + xxhash64(c, 0), + xxhash64(c, n), + hash(array(c)), + xxhash64(array(c)), + hash(struct(c)), + xxhash64(struct(c)) +FROM ( + SELECT * FROM VALUES + (1, make_interval(0, 0, 0, 0, 0, 0, 0)), + (2, make_interval(0, 1, 0, 0, 0, 0, 0)), + (3, make_interval(0, 0, 0, 1, 0, 0, 0)), + (4, make_interval(0, 0, 0, 2, 0, 0, 0)), + (5, make_interval(0, 0, 0, 0, 0, 0, 0.000001)), + (6, make_interval(1, 2, 3, 4, 5, 6, 7.008009)), + (7, make_interval(-1, -2, -3, -4, -5, -6, -7.008009)), + (8, CAST(NULL AS INTERVAL)) + AS t(n, c) + DISTRIBUTE BY n +) --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
