Skip to content

[EPIC] Complete interval type support: scan, operator gates, and remaining interval expressions #5061

Description

@andygrove

What is the problem the feature request solves?

#4540 tracked the foundational interval work: map the three Spark interval types to Arrow and get them through the FFI, serde and codegen-dispatch paths. That part has now largely landed via #4898 (CalendarIntervalType) and #4976 (nested values and native shuffle), so #4540's premise ("Comet has no support for Spark's interval data types") no longer describes the state of the tree. This issue supersedes it and tracks what the audit of 1d3044f35 found still missing.

The headline result: the native engine handles Interval / Duration arrays better than the Scala side admits. Almost every remaining gap is a type gate in Scala, not missing Rust.

Verified working today

All of the following ran fully native and matched Spark on 1d3044f35 (Spark 4.1 profile), against Parquet-backed data so the expressions were not constant-folded:

  • make_dt_interval, make_ym_interval, multiply_dt_interval
  • an interval column through an equi-join, a multi-key sort, first / last, coalesce / if, and =
  • native shuffle carrying an ANSI interval data column

Verified broken or falling back

Area Behavior on 1d3044f35
hash / xxhash64 of a CalendarInterval Query fails: Unsupported data type in hasher: Interval(MonthDayNano)
Null CalendarInterval literal Query fails (#5058)
Parquet scan of an ANSI interval column Entire scan falls back
Single-column sort on an interval Sort falls back (multi-column sort is not checked and works)
min / max / sum / avg over intervals HashAggregate falls back
GROUP BY <interval> Exchange falls back
Window with an interval in PARTITION BY / ORDER BY / the aggregate Window falls back
hash / xxhash64 of an ANSI interval Project falls back
-interval, abs(interval) Project falls back, despite native support existing
cast(interval as string), cast(string as interval) Project falls back
extract / date_part of an interval field Project falls back
date + interval, ts + interval, ts - ts, date - date Project falls back
interval / n, ym_interval * n Project falls back

Describe the potential solution

1. Correctness bugs (do these first)

Both are cases where a Scala gate claims support the native side does not have. Worth a sweep of the other supportedDataType consumers (CometScalarSubquery, CometSink) for the same shape of problem.

2. Scan support

This is the gating item for everything else being reachable in real queries. Today the only way to get an interval column into a Comet operator is to construct it inline with make_dt_interval / make_ym_interval, which is what every existing test does. Note that CalendarIntervalType can never appear in a Parquet file (Spark's ParquetTable.supportsDataType accepts AtomicType; CalendarIntervalType extends DataType directly while the ANSI types extend AnsiIntervalType extends AtomicType), so the current DataTypeSupport.isTypeSupported list is inverted: it admits the one interval type that cannot occur and rejects the two that can.

3. Type gates that still exclude intervals

Each of these is a small change plus a test, and each currently costs a fallback:

  • QueryPlanSerde.supportedScalarSortElementType already carries a TODO: Include SparkSQL's YearMonthIntervalType and DayTimeIntervalType. Single-column sort falls back; multi-column sort is not checked at all and produces correct results, so the restriction looks removable.
  • AggSerde.minMaxDataTypeSupported / avgDataTypeSupported / sumDataTypeSupported reject intervals. Spark supports min / max / sum / avg over both ANSI interval types.
  • CometShuffleExchangeExec.supportedHashPartitioningDataType and the columnar-shuffle supportedSerializableDataType reject all three interval types, so GROUP BY <interval> falls back unless shuffle mode is native. feat: support interval codegen dispatch for nested values and native shuffle #4976 added intervals to the native-shuffle predicate only, leaving the two paths asymmetric.
  • Window operator support for interval partition / order / aggregate expressions.
  • arithmetic.scala's supportedDataType rejects intervals, so UnaryMinus and Abs fall back even though native/spark-expr/src/math_funcs/negative.rs already has overflow-checked interval negation that is unreachable and untested. Tracked as Math expressions: revisit interval / input-type gating and add regression tests once interval support lands #4756.
  • literals.scala special-cases only DayTimeIntervalType; YearMonth and Calendar interval literals are Unsupported.

PR #5025 (closes #5021) centralizes these predicates and is the natural place to land several of them coherently rather than one gate at a time.

4. Expressions not yet wired

Now that the types round-trip, most of these are a one-line CometCodegenDispatch registration plus a SQL file test.

Expression Issue Open PR
make_dt_interval #3098 landed; #4338 predates it
make_ym_interval #3100 landed
multiply_dt_interval #3101 (closed) landed
make_interval #3099 #5030 (dispatch), #5039 (native)
timestampadd / timestampdiff #5030
multiply_ym_interval #3102 #4715
divide_dt_interval #3096 #4901
divide_ym_interval needs an issue
multiply_interval / divide_interval (legacy CalendarInterval * and /) needs an issue
extract / date_part of interval fields (ExtractANSIInterval*, ExtractInterval*) needs an issue
date_add_interval #3086
timestamp_add_interval #3114
timestamp_add_ym_interval #3115
date_add_ym_interval needs an issue
time_add_interval #3121
subtract_timestamps #3112
subtract_dates #3094
subtract_times #3139
datetime_sub #3134
try_make_interval #3103
cast interval to/from string needs an issue

multiply_dt_interval, make_dt_interval and make_ym_interval are done; the rest of the column is the remaining work. Several of the open PRs above were written before the type plumbing landed and may simplify considerably now.

5. Test coverage

Interval coverage today is four SQL files (calendar_interval.sql, make_dt_interval.sql, make_ym_interval.sql, multiply_dt_interval.sql), one CometCodegenSuite round-trip and one CometArrowStreamSuite round-trip. Nothing covers the operator paths that #4898 and #4976 newly unlocked and that I verified do work: join, multi-key sort, first / last, coalesce / if / comparison, and native shuffle of ANSI (not just Calendar) intervals. Those are silent-regression risk right now.

A trap worth documenting for anyone writing these: a single-row VALUES (make_dt_interval(...)) gets collapsed by ConvertToLocalRelation into a folded literal, so the test passes without exercising Comet at all. Derive intervals from Parquet-backed integer columns instead.

6. Docs

  • expressions.md marks extract and date_part as supported with no interval caveat. The avg, try_avg, abs, * and try_add rows already carry accurate "interval falls back" notes.

Additional context

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions