Repository navigation
[EPIC] Complete interval type support: scan, operator gates, and remaining interval expressions #5061
Description
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)
- Hashing a CalendarInterval value fails with "Unsupported data type in hasher: Interval(MonthDayNano)" #5059 —
hash/xxhash64of a CalendarInterval aborts the stage. feat: add CalendarIntervalType support #4898 addedCalendarIntervalTypetoQueryPlanSerde.supportedDataType, which is the gatehash.scalaconsults, butnative/spark-expr/src/hash_funcs/utils.rshas no branch forInterval(MonthDayNano). - Null CalendarInterval literal throws native exception instead of evaluating to null #5058 — a null CalendarInterval literal throws a native exception instead of evaluating to null.
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
- Support reading ANSI interval columns (YearMonthIntervalType / DayTimeIntervalType) in the native Parquet scan #5060 — read ANSI interval columns (
YearMonthIntervalType/DayTimeIntervalType) in the native Parquet scan.
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.supportedScalarSortElementTypealready carries aTODO: 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/sumDataTypeSupportedreject intervals. Spark supportsmin/max/sum/avgover both ANSI interval types. -
CometShuffleExchangeExec.supportedHashPartitioningDataTypeand the columnar-shufflesupportedSerializableDataTypereject all three interval types, soGROUP BY <interval>falls back unless shuffle mode isnative. 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'ssupportedDataTyperejects intervals, soUnaryMinusandAbsfall back even thoughnative/spark-expr/src/math_funcs/negative.rsalready 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.scalaspecial-cases onlyDayTimeIntervalType; YearMonth and Calendar interval literals areUnsupported.
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.
- Operator-level tests for the paths listed above, for all three interval types.
- Scan round-trip tests once Support reading ANSI interval columns (YearMonthIntervalType / DayTimeIntervalType) in the native Parquet scan #5060 lands: nulls, negatives, the full
startField/endFieldrange, dictionary encoding, multiple row groups. - A gate-consistency test, so a type admitted by
supportedDataTypebut unhandled natively is caught at test time rather than as a stage abort. Hashing a CalendarInterval value fails with "Unsupported data type in hasher: Interval(MonthDayNano)" #5059 and Null CalendarInterval literal throws native exception instead of evaluating to null #5058 are both instances of that class.
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.mdmarksextractanddate_partas supported with no interval caveat. Theavg,try_avg,abs,*andtry_addrows already carry accurate "interval falls back" notes.
Additional context
- Supersedes [EPIC] Support Spark interval types (CalendarInterval / YearMonthInterval / DayTimeInterval) and interval expressions #4540, which tracked the foundational type work now delivered by feat: add CalendarIntervalType support #4898 and feat: support interval codegen dispatch for nested values and native shuffle #4976.
- Audit performed against
1d3044f35with the Spark 4.1 profile. Every "verified" claim above was reproduced with temporary SQL-file tests rather than inferred from reading the gates. - Related: [EPIC] Implement all Spark date/time expressions #4418 ([EPIC] date/time expressions), [EPIC] Provide JVM/codegen-dispatch implementations for Incompatible expressions so they never fall back by default #4506 ([EPIC] codegen dispatch), Centralize data-type support predicates so new types don't have to be added in multiple places #5021 / refactor: centralize data type support predicates #5025 (centralizing type predicates).
Activity