apache / apache/datafusion-comet

Null CalendarInterval literal throws native exception instead of evaluating to null

Open
#5,058 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 6h
Merged PRs (30d)
190

Description

### Describe the bug

A null literal of `CalendarIntervalType` in a projection throws a native exception instead of producing a null:

```
org.apache.comet.CometNativeException: General execution error with reason:
Interval(MonthDayNano) is not supported in Comet.
```

#4898 mapped `CalendarIntervalType` to Arrow `Interval(MonthDayNano)` and added it to the data-type support checks, the codegen kernels, and plan serialization. The null-literal path in the native planner was not updated to match. `create_null_literal`'s match in `native/core/src/execution/planner.rs:389-421` handles Utf8, Date32, Timestamp, Decimal128, Struct, Map, List, Time64, Duration and friends, but has no `Interval` arm, so it hits the catch-all at `:418`.

The result is a hard failure rather than a fallback: because the type now passes `isSupportedDataType`, the plan is accepted and only fails once it reaches native execution.

Contrast with `YearMonthIntervalType`, which is not in the supported set and so falls back cleanly:

```
Project [COMET: Unsupported data type YearMonthIntervalType(0,1)]
```

That is the behavior a null calendar interval should have at minimum, and ideally it should just evaluate natively.

### Steps to reproduce

```sql
SELECT CAST(NULL AS INTERVAL);
```

This is easy to hit indirectly. `MakeInterval` is `NullIntolerant`, so `NullPropagation` rewrites any call with a literal NULL argument into a null interval literal, and the original expression disappears before Comet sees it:

```sql
SELECT make_interval(NULL, 2, 3, 4, 5, 6, 7.008009);
```

Both fail with the exception above.

Non-null calendar interval literals are fine, and calendar interval values flowing through a `LocalTableScan` (as in `calendar_interval.sql`) are fine. It is specifically the null literal in a projection.

### Expected behavior

`SELECT CAST(NULL AS INTERVAL)` returns a single null row, matching Spark.

Adding an `Interval(MonthDayNano)` arm producing `ScalarValue::IntervalMonthDayNano(None)` to the null-literal match should cover it. Worth checking `Interval(YearMonth)` and `Interval(DayTime)` at the same time, since they are absent from that match too, though they are currently shielded by the type check.

### Additional context

Found while adding `make_interval` codegen-dispatch support in #5030. That PR works around it by driving its NULL coverage from nullable columns rather than NULL literals, since literal NULL arguments are constant-folded away by `NullPropagation` and never exercise the expression anyway.

Related: #4540 (interval type support epic), #4898 (the PR that added `CalendarIntervalType`).

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.