[SPARK-57832][SQL] Support timestamp - timestamp subtraction for nanosecond-precision timestamps - #57680
Conversation
…second-precision timestamps ### What changes were proposed in this pull request? `SubtractTimestamps` (the `timestamp - timestamp` operator) previously only accepted the microsecond timestamp types (`AnyTimestampType`). This PR extends it to also accept the nanosecond-precision timestamp types (`TIMESTAMP_NTZ(p)` / `TIMESTAMP_LTZ(p)`, `p` in [7, 9]). The difference is always reported on the microsecond grid, since the result is a `DayTimeIntervalType` (which has microsecond resolution). Each operand therefore contributes only its `epochMicros`; the sub-microsecond remainder is truncated. This keeps `ts_nanos - ts_nanos` numerically identical to `ts_nanos::timestamp - ts_nanos::timestamp`. Concretely: - `SubtractTimestamps.inputTypes` widens to `TypeCollection(AnyTimestampType, AnyTimestampNanoType)` for both operands, and `nullSafeEval` / `doGenCode` reduce a `TimestampNanosVal` operand to its `epochMicros` before subtracting. - `BinaryArithmeticWithDatetimeResolver` recognizes a nanos operand on either side of a `Subtract` and routes it to `SubtractTimestamps`. - The `DateTimeOperations` / `AnsiDateTimeOperations` type-coercion rules unify mixed operands (DATE vs timestamp, or two timestamps differing in precision or time-zone family). A cross-family pair unifies in the NTZ family (mirroring the microsecond `TIMESTAMP - TIMESTAMP_NTZ` precedent); a same-family pair keeps its family so an LTZ pair still subtracts in the session time zone. For pure-microsecond inputs the behavior is unchanged. ### Why are the changes needed? Sub-task of SPARK-56822 (nanosecond-precision timestamp support). Without this, subtracting two nanosecond timestamps fails analysis. ### Does this PR introduce _any_ user-facing change? Yes. `timestamp_nanos - timestamp_nanos` (and mixed micro/nanos, and nanos/DATE) now produces a microsecond-grid `DayTimeIntervalType` instead of failing analysis. ### How was this patch tested? - New unit test in `DateExpressionsSuite` covering NTZ/LTZ nanos operands, sub-microsecond truncation, pre-epoch values, NULLs, and interpreted/codegen consistency. - New coercion cases in `TypeCoercionSuite` / `AnsiTypeCoercionSuite`. - New golden SQL cases in `timestamp-ntz-nanos.sql` / `timestamp-ltz-nanos.sql`. Co-authored-by: Isaac
bdc6f8b to
45048af
Compare
|
cc @uros-b , @cloud-fan can you PTAL at this PR. |
…amps requiredType Widening SubtractTimestamps.inputTypes to TypeCollection(AnyTimestampType, AnyTimestampNanoType) changes the DATATYPE_MISMATCH.UNEXPECTED_INPUT_TYPE "requiredType" message from the single type to the collection form. Regenerate the four pre-existing golden files that exercise that error path (results and analyzer-results): nonansi/timestamp, timestampNTZ/timestamp, time, and typeCoercion/native/decimalPrecision. Co-authored-by: Isaac
uros-b
left a comment
There was a problem hiding this comment.
Some test coverage gaps, for completeness:
- DATE ↔ TIMESTAMP_LTZ(p) coercion (only NTZ DATE cases are tested)
- Micro TIMESTAMP ↔ TIMESTAMP_NTZ(p) (cross-family micro/nanos)
- LTZ SQL has no DATE operand case (NTZ does)
- etc.
cloud-fan
left a comment
There was a problem hiding this comment.
0 blocking, 1 non-blocking, 4 nits.
The implementation path looks sound; the remaining items are inaccurate comments and a focused coercion-test coverage gap.
Already raised in existing discussion (1)
- The LTZ SQL comment incorrectly calls subtraction zone-agnostic even though the non-legacy path uses session-zone local datetimes. -- existing discussion
Nits: 3 minor items (see inline comments).
Suggestions (1)
- General: Add focused DATE/LTZ-nanos and cross-family micro/nanos coercion cases so the distinct casting and family-selection arms are covered in both directions.
Verification
I traced nanos subtraction from arithmetic resolution through ANSI/non-ANSI operand coercion into both interpreted and generated evaluation. Both evaluation modes reduce TimestampNanosVal to epochMicros; LTZ non-legacy subtraction continues through the existing session-zone-aware DateTimeUtils.subtractTimestamps path, while legacy mode returns CalendarIntervalType.
HyukjinKwon
left a comment
There was a problem hiding this comment.
0 blocking, 0 non-blocking, 3 nits.
Sound nanosecond-subtraction coercion; only minor comment-accuracy nits (already raised) and a non-blocking coverage suggestion remain.
Already raised in existing discussion (3)
- The LTZ nanos SQL comment says subtraction 'is zone-agnostic, so the session time zone does not change the interval magnitude.' For LTZ non-legacy, subtractTimestamps uses session-zone local datetimes, so DST can change the interval (same as micro TIMESTAMP). The comment is inaccurate. -- existing discussion
- subtractTimestampsCommonType hardcodes getOrElse(6) for the micro precision while the nearby findWiderDateTimeType uses a named MicrosPrecision = 6 constant; using the same named constant would be more consistent. -- existing discussion
- The SubtractTimestamps comment says the difference 'always ... a DayTimeIntervalType', but in legacy-interval mode dataType is CalendarIntervalType (the def right below shows both). The microsecond-grid property is common to both modes; the concrete type is config-dependent. -- existing discussion
Verification
Traced subtractTimestampsCommonType widening + family selection, and confirmed nullSafeEval/toMicros and doGenCode/toMicrosCode reduce operands identically to epochMicros with the legacyInterval branch preserved on both. Spot-checked new golden outputs (timestamp-ntz-nanos.sql.out precision p7/8/9). Build/test: build/sbt 'catalyst/testOnly *TypeCoercionSuite *DateExpressionsSuite' and the CSV/timestamp SQLQueryTestSuite goldens.
…cion test coverage Follow-up to review comments on the nanosecond timestamp subtraction PR: - Fix the LTZ SQL comment that called the subtraction "zone-agnostic"; the non-legacy path subtracts session-zone local date-times, so DST can shift the interval (same as micro TIMESTAMP). - Hoist MicrosPrecision = 6 to a shared TypeCoercionHelper constant and use it in findWiderDateTimeType and subtractTimestampsCommonType. - Note in SubtractTimestamps docs that nanos operands are accepted (truncated to epochMicros). - Qualify the "always DayTimeIntervalType" comments: the difference is always on the microsecond grid, but the concrete type is CalendarIntervalType under spark.sql.legacy.interval.enabled. - Correct the DateExpressionsSuite comment describing which operand pair yields the zero result. - Add DATE<->LTZ-nanos and cross-family micro<->nanos coercion cases, and a DATE-operand case in timestamp-ltz-nanos.sql (goldens regenerated). Co-authored-by: Isaac
cloud-fan
left a comment
There was a problem hiding this comment.
5 addressed, 0 remaining, 0 new to this AI review.
0 blocking, 0 non-blocking, 0 nits.
The current implementation is coherent across resolution, coercion, and evaluation, and the prior review concerns are addressed.
Verification
Traced SQL subtraction from BinaryArithmeticWithDatetimeResolver through ANSI/non-ANSI datetime coercion into SubtractTimestamps. Confirmed both interpreted and generated paths extract epochMicros, the legacy and non-legacy result branches remain intact, and the focused coercion tests plus SQL goldens exercise the added operand families and previously missing combinations.
What changes were proposed in this pull request?
SubtractTimestamps(thetimestamp - timestampoperator) previously only accepted the microsecond timestamp types (AnyTimestampType). This PR extends it to also accept the nanosecond-precision timestamp types (TIMESTAMP_NTZ(p)/TIMESTAMP_LTZ(p),pin [7, 9]).The difference is always reported on the microsecond grid, since the result is a
DayTimeIntervalType(which has microsecond resolution). Each operand therefore contributes only itsepochMicros; the sub-microsecond remainder is truncated. This keepsts_nanos - ts_nanosnumerically identical tots_nanos::timestamp - ts_nanos::timestamp.Concretely:
SubtractTimestamps.inputTypeswidens toTypeCollection(AnyTimestampType, AnyTimestampNanoType)for both operands, andnullSafeEval/doGenCodereduce aTimestampNanosValoperand to itsepochMicrosbefore subtracting.BinaryArithmeticWithDatetimeResolverrecognizes a nanos operand on either side of aSubtractand routes it toSubtractTimestamps.DateTimeOperations/AnsiDateTimeOperationstype-coercion rules unify mixed operands (DATE vs timestamp, or two timestamps differing in precision or time-zone family). A cross-family pair unifies in the NTZ family (mirroring the microsecondTIMESTAMP - TIMESTAMP_NTZprecedent); a same-family pair keeps its family so an LTZ pair still subtracts in the session time zone.For pure-microsecond inputs the behavior is unchanged.
Why are the changes needed?
Sub-task of SPARK-56822 (nanosecond-precision timestamp support). Without this, subtracting two nanosecond timestamps fails analysis.
Does this PR introduce any user-facing change?
Yes.
timestamp_nanos - timestamp_nanos(and mixed micro/nanos, and nanos/DATE) now produces a microsecond-gridDayTimeIntervalTypeinstead of failing analysis.How was this patch tested?
DateExpressionsSuitecovering NTZ/LTZ nanos operands, sub-microsecond truncation, pre-epoch values, NULLs, and interpreted/codegen consistency.TypeCoercionSuite/AnsiTypeCoercionSuite.timestamp-ntz-nanos.sql/timestamp-ltz-nanos.sql.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 4.8)