[WIP][SPARK-57833][SQL] Support nanosecond-precision timestamps in timestampdiff - #58880
Open
stevomitric wants to merge 3 commits into
Open
stevomitric wants to merge 3 commits into
stevomitric wants to merge 3 commits into
Conversation
…mpadd ### What changes were proposed in this pull request? Extend `timestampadd(unit, quantity, timestamp)` to accept nanosecond-precision timestamp endpoints (`TIMESTAMP_NTZ(p)` / `TIMESTAMP_LTZ(p)`, p in [7, 9]). - Widen `TimestampAdd.inputTypes` to `TypeCollection(AnyTimestampType, AnyTimestampNanoType)` so a nanosecond argument is no longer implicitly cast to microseconds (which silently dropped the sub-microsecond fraction). The result keeps the input's nanosecond type. - For units of MICROSECOND or coarser, the addition runs on the microsecond grid (reusing the existing calendar/DST/overflow handling) and the sub-microsecond fraction is carried through unchanged. - Add a `NANOSECOND` unit (the token already exists for interval literals; it is added to the shared `datetimeUnit` grammar rule): the fraction absorbs the quantity and whole microseconds carry into `epochMicros`. Overflow surfaces as `DATETIME_OVERFLOW`. `NANOSECOND` is meaningful only for a nanosecond-precision timestamp; on a microsecond timestamp it is rejected as an invalid unit. The new logic lives in `TimestampAdd` and a new `DateTimeUtils.timestampAddNanos` helper. `timestampdiff` is out of scope here: because `datetimeUnit` is shared, `timestampdiff(NANOSECOND, ...)` now parses but returns a clean `INVALID_PARAMETER_VALUE.DATETIME_UNIT` error at runtime (previously an `UNRESOLVED_ROUTINE`); nanosecond support for `timestampdiff` is left to the rest of SPARK-57833. ### Why are the changes needed? Part of the nanosecond-precision timestamp umbrella (SPARK-56822). Without this, `timestampadd` rejected nanosecond timestamp arguments by casting them to microseconds, silently losing the sub-microsecond digits. ### Does this PR introduce any user-facing change? Yes. `timestampadd` now accepts nanosecond-precision timestamps and preserves their sub-microsecond fraction, and supports a new `NANOSECOND` unit for nanosecond-precision inputs. ### How was this patch tested? New unit test in `DateExpressionsSuite` (NTZ/LTZ, precisions 7/8/9, fraction preservation for units >= MICROSECOND, the NANOSECOND unit with positive/negative and multi-microsecond carry, case-insensitivity, null propagation, the invalid-unit rejection on a microsecond timestamp, and carry overflow; interpreted and codegen via checkEvaluation). Added `timestampadd` cases to the `timestamp-ntz-nanos.sql` / `timestamp-ltz-nanos.sql` golden files and regenerated the analyzer and result goldens. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Isaac Co-authored-by: Isaac <no-reply@databricks.com>
… input's precision Addresses the review of PR apache#58855. Correctness fix (blocking): the NANOSECOND branch of `timestampAddNanos` computed the remainder at full nanosecond resolution without flooring to the input's declared precision `p`. On a `TIMESTAMP_NTZ/LTZ(7)` or `(8)` value that produced an off-grid sub-precision fraction that displays identically to the aligned value but compares, hashes and orders unequal (silent wrong results in =/DISTINCT/GROUP BY/joins/sort). The precision `p` is now threaded into `timestampAddNanos` from the expression's declared type and the NANOSECOND result is floored to `p` via `truncateTimestampNanosToPrecision`. Also: - Fix a spurious overflow rejection: split the added nanoseconds into whole microseconds and a [0, 999] remainder before combining, so a large but representable NANOSECOND quantity (near `Long.MaxValue`) is no longer rejected by an intermediate `addExact(nanosWithinMicro, quantity)` that could not fit a Long even though the true microsecond result does. - Add `DateExpressionsSuite` cases exercising NANOSECOND on `p = 7`/`8` (flooring to the 100ns / 10ns step, including carry) and a near-`Long.MaxValue` representable quantity; assert the overflow case raises `DATETIME_OVERFLOW`. - Add end-to-end golden coverage: a `p = 7` flooring case in `timestamp-ntz-nanos.sql`, carry and invalid-unit cases in `timestamp-ltz-nanos.sql`, and a `timestampdiff(NANOSECOND, ...)` case in `timestamp.sql` documenting that the shared `datetimeUnit` keyword is rejected at runtime by `timestampdiff` (matching add-only units like DAYOFYEAR). - Document the NANOSECOND flooring behavior in the function description and make `isNanos` a `@transient lazy val`. Co-authored-by: Isaac <no-reply@databricks.com>
…mpdiff TimestampDiff previously accepted only microsecond TimestampType operands. This extends it to nanosecond-precision timestamps (TIMESTAMP_NTZ/LTZ(p), p in [7, 9]): when either operand is a nanosecond carrier, the difference is computed at full nanosecond resolution so each operand's sub-microsecond fraction participates in the truncated unit count (for every unit, not only NANOSECOND). The NANOSECOND unit is added to the shared unit map, so it is now accepted by timestampdiff for both nanosecond and microsecond operands (microsecond operands carry a zero fraction). - inputTypes widened to TypeCollection(AnyTimestampType, AnyTimestampNanoType) for both operands. - New DateTimeUtils.timestampDiffNanos folds each operand's nanosWithinMicro fraction into a nanosecond LocalDateTime before taking the difference. - Interpreted and codegen paths read epochMicros / nanosWithinMicro from the nanosecond carrier and pass a zero fraction for microsecond operands. - Adds catalyst unit tests and golden-file coverage. Co-authored-by: Isaac <no-reply@databricks.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Extends
timestampdiffto nanosecond-precision timestamps (TIMESTAMP_NTZ/LTZ(p)p in [7, 9]). When either operand is a nanosecond carrier, the difference is computed at full nanosecond resolution so each operand's sub-microsecond fraction participates in the truncated unit count — for every unit, not onlyNANOSECOND. TheNANOSECONDunit is added to the shared unit map, sotimestampdiff(NANOSECOND, …)now works for both nanosecond and microsecond operands (microsecond operands carry a zero fraction).Why are the changes needed?
Part of SPARK-57833, nanosecond-precision timestamp support
Does this PR introduce any user-facing change?
Yes.
timestampdiffnow accepts nanosecond-precision timestampHow was this patch tested?
New UT + sql golden file tests.
Was this patch authored or co-authored using generative AI tooling?
Co-Authored-by: Claude Opus 4.8