Repository navigation
Math expressions: revisit interval / input-type gating and add regression tests once interval support lands #4756
Description
Spun out from items #4 and #7 of #4500 to defer until the in-flight native interval-support PRs land. Both are math expressions whose interval handling becomes relevant only once interval types serialize.
greatest / least (#4500 item 4)
Greatest / Least are registered as bare CometScalarFunction("greatest") / CometScalarFunction("least") with no input-type gating.
Probed on Spark 3.5 and 4.0 across a wide range of input types, comparing Comet output against Spark and recording whether the projection ran natively:
| Input type | Runs natively | Matches Spark |
|---|---|---|
| int / mixed numeric | yes | yes |
double incl. NaN / -0.0 / Infinity |
yes | yes |
| decimal (mixed precision/scale) | yes | yes |
| string (default collation) | yes | yes |
| date / timestamp | yes | yes |
| binary | yes | yes |
| array (incl. with-null, nested, array-of-struct) | yes | yes |
| struct (incl. with-null) | yes | yes |
| null-only | yes | yes |
| day-time / year-month interval | no (falls back) | yes |
collated string (Spark 4.x, e.g. UTF8_LCASE) |
no (falls back) | yes |
No live defect today. Unsupported types (intervals, collated strings) fail serialization and fall back to Spark cleanly: no runtime error, no silent divergence. Note: the original suggestion to gate to scalar sort element types via supportedScalarSortElementType would be a net regression, since it would disable the array / struct / decimal cases that currently work natively and correctly.
signum (#4500 item 7)
Spark Signum accepts DoubleType, YearMonthIntervalType, DayTimeIntervalType (returning a Double sign of the underlying months / micros). Comet registers a bare CometScalarFunction("signum") whose native UDF only handles double.
Probed on Spark 3.5:
| Input type | Runs natively | Matches Spark |
|---|---|---|
| double | yes | yes |
| int (cast to double) | yes | yes |
| day-time interval | no (falls back) | yes |
| year-month interval | no (falls back) | yes |
No live defect today. Interval inputs fall back gracefully (intervals do not serialize), so there is no runtime error.
Why this is deferred
There are pending PRs adding native interval support. Once intervals serialize, both greatest / least and signum over interval columns will start reaching their native UDFs instead of falling back, so the correctness / gating story must be re-evaluated at that point. For signum, the native path would additionally need to read the interval's integer representation (months for year-month, micros for day-time) and return its sign as a double.
Deferred work
- After interval support lands, re-verify
greatest/leastoverDayTimeIntervalType/YearMonthIntervalType(correctness vs Spark; add agetSupportLevelgate only if a real divergence/error appears). - After interval support lands, handle
signumover interval inputs: either extend the nativesignumpath to compute the sign of the underlying integer representation, or add an explicitgetSupportLevelgate so it falls back cleanly with a reason in EXPLAIN. - Add
CometExpressionSuiteregression tests locking in the verified native behavior for both expressions (numerics/NaN, decimal, string, date, timestamp, binary, arrays incl. nested/with-null, structs incl. with-null, null-only for greatest/least; double/int for signum) plus the graceful interval / collated-string fallback. - Revisit collated-string handling for
greatest/leastunder the collation umbrella (Do another audit sweep for string collation differences #4496) if collated inputs ever become natively supported.
Surfaced by the audit-comet-expression skill run in #4486.
Activity
[-]greatest / least: add regression tests and revisit input-type gating once interval support lands[/-][+]Math expressions: revisit interval / input-type gating and add regression tests once interval support lands[/+]