apache / apache/datafusion-comet

[Bug] Preserve CalendarInterval microseconds across Comet boundaries and native kernels

Open
#5,279 1 comment 0 reactions 1 assignee Claimed by @peterxcli View on GitHub
area:expressions bug priority:high
Dominant language
Scala
Stars
1.3k
Forks
373
Avg merge
2d 4h
Merged PRs (30d)
198

Description

## Describe the bug

Comet maps Spark's `CalendarIntervalType` representation (`months: i32`, `days: i32`, `microseconds: i64`) to Arrow `IntervalMonthDayNano` (`months: i32`, `days: i32`, `nanoseconds: i64`). The JVM codegen-dispatch output path therefore multiplies `CalendarInterval.microseconds` by 1,000 before writing it to an `IntervalMonthDayNanoVector`.

That conversion limits otherwise valid Spark elapsed-time components to approximately 292 years. This limit applies to the microseconds component expressed through hours, minutes, or seconds, not to calendar years stored separately as months. For larger elapsed-time values, the default dispatch path aborts with an uncaught `ArithmeticException: long overflow`, even when ANSI mode is disabled.

The native path has a second, independent compatibility problem. Comet currently wraps `datafusion-spark`'s `SparkMakeInterval`, which:

- hardcodes its return type as Arrow `Interval(MonthDayNano)`;
- computes hours, minutes, and seconds in nanoseconds, imposing the same approximately 292-year elapsed-time limit; and
- accepts seconds as `Float64`, so it cannot preserve Spark's exact `Decimal(18,6)` microsecond semantics.

Introducing a lossless boundary representation alone therefore will not make native `make_interval` compatible. The native kernel must also be replaced or extended to produce the new representation using exact microsecond arithmetic.

## Steps to reproduce

Run without `spark.comet.expression.MakeInterval.allowIncompatible` so `MakeInterval` uses the default JVM codegen-dispatch path:

```sql
CREATE TABLE test_make_interval_dispatch(hours INT) USING parquet;
INSERT INTO test_make_interval_dispatch VALUES (2562048);
SELECT make_interval(0, 0, 0, 0, hours) FROM test_make_interval_dispatch;
```

Spark returns a valid `CalendarInterval`. Comet aborts while writing the result to `IntervalMonthDayNanoVector` because `Math.multiplyExact(interval.microseconds, 1000L)` overflows.

## Expected behavior

The complete fix has two parts.

### 1. Represent `CalendarIntervalType` losslessly across Comet boundaries

Use the Spark-compatible components:

```text
months: i32
days: i32
microseconds: i64
```

A tagged Arrow struct is one possible representation. The implementation must keep calendar days separate from elapsed microseconds because a calendar day can be 23, 24, or 25 hours across DST transitions.

Update the representation consistently across the protobuf type mapping, FFI, serde, JVM and native writers/readers, codegen dispatch, native shuffle, and any native expressions that consume or produce `CalendarIntervalType`.

### 2. Make the native interval kernel use the lossless representation

Replace or extend the imported `datafusion-spark` `SparkMakeInterval` kernel so it:

- returns the new Spark-compatible calendar interval representation instead of `IntervalMonthDayNano`;
- computes elapsed time directly in microseconds without an intermediate nanosecond conversion;
- preserves `Decimal(18,6)` seconds exactly without coercing to `Float64`;
- implements Spark-compatible overflow and NULL behavior for ANSI, non-ANSI, and `try_make_interval`; and
- supports the full Spark range for months, days, and elapsed microseconds.

Only after both parts land should Comet remove `spark.comet.expression.MakeInterval.allowIncompatible` and the related ignored tests.

Coverage should include boundary round trips, JVM dispatch, native execution, native shuffle, NULL inputs, negative values, microsecond precision, extreme elapsed-time values, ANSI overflow behavior, and `try_make_interval`.

## Additional context

- PR: #5039
- Default dispatch range-limit review: https://github.com/apache/datafusion-comet/pull/5039#discussion_r3722969753
- Native precision/range issue: #5131
- Spark precedent: https://github.com/apache/spark/commit/5ca6b1062887664b16b11f2bfcc015c9e616dc49

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.