LuciferYang opened a new pull request, #68148:
URL: https://github.com/apache/doris/pull/68148
### What problem does this PR solve?
Issue Number: close #68118
Problem Summary:
The v1 Parquet reader turns a TIMESTAMP column's min/max into a zone map
without the safety the v2 reader has, so pruning can drop matching rows. Both
defects are in `ParquetPredicate::parse_min_max_value`
(`be/src/format/parquet/parquet_predicate.h`), v1 only
(`enable_file_scanner_v2` defaults true).
An adjusted-to-UTC INT64 timestamp mapped to DATETIMEV2 has its physical
min/max converted to local civil time. That conversion is not monotonic across
a backward clock transition: in `America/New_York`, UTC `[05:30, 06:30]`
converts to civil `[01:30, 01:30]`, while an interior `05:59` converts to
`01:59`, outside the converted range. `dt > '2021-11-07 01:45:00'` then prunes
a row group that has a matching row. The v2 reader fences this with
`timestamp_min_max_is_safe` / `utc_timestamp_range_is_monotonic`; v1 had no
equivalent.
Separately, the INT96 / DATETIMEV2 branch read both local operands from
`min_field`:
```cpp
auto min_value = min_field->get<TYPE_DATETIMEV2>();
auto max_value = min_field->get<TYPE_DATETIMEV2>();
...
if (min_value != max_value) { return Status::DataQualityError(...); }
```
so the `min_value != max_value` check compared a value with itself and never
fired. That check exists to reject legacy INT96 stats whose ordering is
unreliable (PARQUET-1065). The footer path is still covered by
`read_column_stats`, which rejects unequal INT96 stats before this point; the
page-index path reaches `parse_min_max_value` directly and was left exposed.
The `INT96 || DATETIMEV2` branch is split in two. The output fields
`min_field` / `max_field` are set correctly upstream, so only the validation
changes.
- INT96: reject when the raw encoded bounds differ (`encoded_min !=
encoded_max`), comparing physical bytes rather than converted values. A
single-point range cannot span a transition, so it needs no clock fence.
- DATETIMEV2 (reached only by INT64 physical): a new helper
`adjusted_utc_timestamp_range_is_monotonic` mirrors the unit / adjusted-to-UTC
derivation in `TimestampConverter::init` (including the legacy `converted_type`
TIMESTAMP_MILLIS / MICROS), decodes the raw INT64 range, and defers the
transition check to the shared v2 helper
`format::utc_timestamp_range_is_monotonic`. A non-adjusted timestamp is
displayed in UTC (no transitions) and stays usable; TIMESTAMPTZ never reaches
this branch, so it is exempt, matching v2.
An unusable range returns `DataQualityError`, which every caller already
turns into no zone map / no pruning, not a scan error.
### Release note
Fixed a bug where reading a Parquet TIMESTAMP column with the v1 scanner
could drop matching rows when the min/max range crossed a daylight-saving clock
rollback.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [X] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
`ParseMinMaxRejectsTimestampRangeCrossingClockRollback` drives
`parse_min_max_value` with a synthetic adjusted-to-UTC INT64 timestamp schema
under `America/New_York`: a range crossing the 2021-11-07 fall-back is
rejected, a transition-free range that day stays usable, a spring-forward range
is not rejected for crossing the gap, and the same fall-back range stays usable
when the column is not adjusted to UTC. `ParseMinMaxAppliesInt96SingletonRule`
checks that equal INT96 encoded bounds stay usable and unequal ones are
rejected. Both were mutation-checked: neutering the clock fence fails the
first, neutering the INT96 comparison fails the second.
The end-to-end path the issue asks for, a v1 scan
(`enable_file_scanner_v2=false`, `time_zone='America/New_York'`) over a real
Parquet file whose row group and page index span the rollback, needs a
checked-in Parquet artifact this change cannot synthesize; the unit tests pin
the parser directly, and the file-backed regression is a follow-up.
### Behavior changed:
- [ ] No.
- [X] Yes.
A v1 Parquet TIMESTAMP min/max range that crosses a backward clock
transition, and an INT96 range with differing bounds reached through the page
index, no longer drive pruning. Ranges that were already usable are unaffected.
### Does this need documentation?
- [X] No.
--
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]
