morningman opened a new pull request, #69697:
URL: https://github.com/apache/doris/pull/69697
### What problem does this PR solve?
Issue Number: None
Related PR: #69581 #68516
Problem Summary:
**In short.** An Arrow timestamp whose civil year is outside 0000..9999 is
read into DATETIMEV2 or TIMESTAMPTZ as a different, valid-looking value:
timestamp(us) 2005980768000000000, which is 65537-01-01, reads as a non-null
0001-01-01. This PR checks the year before the value is narrowed into Doris'
16-bit year field, as the ORC path already does, and fails the read otherwise.
The review of #69581 found it on the Lance path; the code is shared by every
Arrow reader.
**Background.** `DataTypeDateTimeV2SerDe::read_column_from_arrow` and
`DataTypeTimeStampTzSerDe::read_column_from_arrow` decode Arrow timestamps for
ADBC, Arrow stream load, paimon-rust, remote Doris and, once #69581 lands,
Lance, nested leaves included. Both convert epoch seconds with
`DateV2Value::from_unixtime()`, which passes cctz's 64-bit year to
`unchecked_set_time(uint16_t year, ...)` ("there's no overflow check since it's
hot path"). #68516 added a check before this narrowing to the ORC path
(`orc_timestamp_to_datetime()`).
**The problem, and what it cost.**
| Arrow timestamp(us) | DATETIMEV2 before | TIMESTAMPTZ before |
|---|---|---|
| 253402300800000000 (10000-01-01) | stored as year 10000, outside the
type's range | error |
| 2005980768000000000 (65537-01-01) | 0001-01-01 00:00:00 | 0001-01-01
00:00:00+00:00 |
| a year before 0000 | wraps to 655xx or back into 0000..9999 | error, or a
wrapped valid year |
In the wrapped cases the read succeeds with a wrong value, and nothing in
the result tells the user.
**How this PR fixes it.**
- DATETIMEV2: convert to civil time in the value's time zone (UTC for a
timezone-naive Arrow timestamp, the session time zone for a zoned one) and
reject a year outside 0000..9999 before packing, like
`orc_timestamp_to_datetime()`.
- TIMESTAMPTZ is stored in UTC, so
`append_timestamptz_from_utc_epoch_micros` checks the instant against
0000-01-01 00:00:00 UTC .. 9999-12-31 23:59:59.999999 UTC before converting.
Its `is_valid_date()` check after packing can no longer fail and becomes a
DCHECK. The message now says 0000-9999, the range `is_valid_date()` already
accepted.
- Both fail the read with DataQualityError, as the other range errors of
these readers do. NULL slots are still skipped before conversion.
- Not changed: the Parquet INT64 / INT96 timestamp paths validate the UTC
range before packing (`parquet_timestamp_micros()`,
`parquet_int96_timestamp_micros()`), so they could not wrap. Arrow reads into
the deprecated DATE / DATETIME (v1) types go through
`VecDateTimeValue::from_unixtime()` with the same unchecked narrowing; they are
left as they are.
**Results.**
| Arrow timestamp(us) | DATETIMEV2 after | TIMESTAMPTZ after |
|---|---|---|
| 253402300799999999 (9999-12-31 23:59:59.999999) | read | read |
| -62167219200000000 (0000-01-01) | read | read |
| 10000-01-01, 65537-01-01, -62167219200000001 (year -1) | DataQualityError
| DataQualityError |
| zoned, at +08:00: 253402271999999999 / 253402272000000000 | 9999-12-31
23:59:59.999999 / DataQualityError | |
### Release note
Reading an Arrow timestamp whose year is outside 0000..9999 into DATETIMEV2
or TIMESTAMPTZ (ADBC, Arrow stream load, paimon-rust, remote Doris) now fails
instead of returning a different date.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- New
`DataTypeDateTimeV2SerDeTest.RejectsArrowTimestampsOutsideDateTimeV2Range`,
`RejectsNestedArrowTimestampOutsideDateTimeV2Range` (an ARRAY leaf) and
`DataTypeTimeStampTzSerDeTest.RejectsArrowTimestampsOutsideTimestampTzRange`.
With the previous DATETIMEV2 code both DATETIMEV2 tests fail; with the previous
TIMESTAMPTZ code 65537-01-01 reads as a valid value and its test fails.
- These suites and the Arrow / Parquet SerDe suites pass on macOS
arm64 (ASAN), run on a tree that also carries #69581, whose Lance reader suites
pass with it (279 tests).
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change -->
Such reads now fail with DataQualityError instead of returning a wrong
date.
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
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]
