MaxGekk commented on code in PR #56562:
URL: https://github.com/apache/spark/pull/56562#discussion_r3431904026
##########
sql/core/src/test/resources/sql-tests/inputs/timestamp-ntz-nanos.sql:
##########
@@ -139,3 +139,8 @@ SELECT array(DATE '2020-01-01') :: array<timestamp_ntz(9)>;
SELECT map('k', TIMESTAMP_NTZ '2020-01-01 12:30:15.123456789') :: map<string,
date>;
SELECT map(DATE '2020-01-01', 'v') :: map<timestamp_ntz(9), string>;
SELECT named_struct('f', DATE '2020-01-01') :: struct<f: timestamp_ntz(9)>;
+
+-- SPARK-57501: TIMESTAMP_NTZ(p) +/- ANSI day-time interval preserves nanos
remainder.
+SELECT TIMESTAMP_NTZ '2020-01-02 03:04:05.123456789' + INTERVAL '2
00:03:00.000456' DAY TO SECOND;
+SELECT TIMESTAMP_NTZ '2020-01-02 03:04:05.123456789' - INTERVAL '1
00:04:00.000321' DAY TO SECOND;
+SELECT TIMESTAMP_NTZ '1960-01-02 03:04:05.123456789' + INTERVAL '0
00:00:00.000001' DAY TO SECOND;
Review Comment:
Good call. Added SQL negative tests to `timestamp-ntz-nanos.sql` and
`timestamp-ltz-nanos.sql`.
One subtlety worth noting: `+ INTERVAL '1' MONTH` is a year-month interval,
which is rejected earlier with `DATATYPE_MISMATCH.BINARY_OP_DIFF_TYPES` and
never reaches `TimestampAddInterval`, so it doesn't exercise the
calendar-interval branch this PR adds. To hit that branch I used a real
`CalendarIntervalType` operand via `make_interval(...)`:
```sql
SELECT TIMESTAMP_NTZ '2020-01-02 03:04:05.123456789' + make_interval(0, 1,
0, 2, 0, 0, 0);
-- DATATYPE_MISMATCH.UNEXPECTED_INPUT_TYPE, requiredType = "INTERVAL DAY TO
SECOND"
```
I kept the year-month case as well, so the goldens now document that nanos
timestamps accept only ANSI day-time intervals. (Note `INTERVAL 1 MONTH 2 DAY`
can't be used here as it's a parse error under default settings.)
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala:
##########
Review Comment:
Done. Added `test("SPARK-57501: timestamp nanos add day-time interval
preserves nanosWithinMicro")` to `DateTimeUtilsSuite`, mirroring the existing
`timestampAddDayTime` test: the DST transition, all `outstandingZoneIds`,
sub-microsecond remainder boundaries (0/1/999), a +1-microsecond carry that
moves only `epochMicros`, a pre-epoch value, and a direct check that
`epochMicros` matches `timestampAddDayTime(...)` while the `nanosWithinMicro`
remainder is carried through unchanged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
