[Bug] v1 Parquet timestamp min/max stats can drive non-conservative pruning (no DST-rollback fence; dead INT96 min==max check)

[LuciferYang]

Search before asking

• I had searched in the issues and found no similar issues.

Version

master, dea1b99.

What's Wrong?

The v1 Parquet reader builds a column zone map from a TIMESTAMP column's min/max statistics without the monotonicity fence the v2 reader applies, and a validation guard for legacy INT96 stats is dead code. Both let non-conservative bounds drive predicate pruning, which drops matching rows.

1. No DST-rollback fence on the v1 timestamp range.

ParquetPredicate::parse_min_max_value (be/src/format/parquet/parquet_predicate.h) converts the physical min/max of an adjusted-to-UTC TIMESTAMP into local civil time using the session time zone, and the v1 slot-zone-map builder in be/src/format/parquet/vparquet_reader.cpp records the result. Conversion to local civil time is not monotonic across a DST fall-back: in America/New_York, UTC [05:30, 06:30] maps to local [01:30, 01:30], while an interior row at UTC 05:59 maps to 01:59, which is outside the converted range. The converted [min, max] is therefore not a valid bound, and a predicate such as dt > 01:45 prunes the row group even though the 01:59 row satisfies it.

The v2 reader already guards this. timestamp_min_max_is_safe (be/src/format_v2/parquet/parquet_statistics.cpp:294-309) calls utc_timestamp_range_is_monotonic (be/src/format_v2/timestamp_statistics.h:40) and treats a non-monotonic range as unusable. The v1 path has no equivalent.

2. The INT96 min == max validation never runs.

The INT96 / DATETIMEV2 branch of parse_min_max_value reads both local values from min_field:

} else if (col_schema->parquet_schema.type == tparquet::Type::type::INT96 ||
           logical_prim_type == TYPE_DATETIMEV2) {
    auto min_value = min_field->get<TYPE_DATETIMEV2>();
    auto max_value = min_field->get<TYPE_DATETIMEV2>();   // reads min_field
    ...
    if (min_value != max_value) {
        return Status::DataQualityError("invalid min/max value");
    }
}

be/src/format/parquet/parquet_predicate.h:344-345. These are local copies; the output fields *min_field / *max_field are set correctly earlier at :309-310, so the recorded bounds are not affected. The effect is that the min_value != max_value check compares a value with itself and can never fire, so the PARQUET-1065 rejection of legacy INT96 stats whose min and max are unreliable (the comment right below it explains why only min == max INT96 stats are trustworthy) is dead. A legacy INT96 file with min != max is used for pruning instead of being rejected.

Reading max_field in that second line is not a complete fix on its own: the branch condition also matches INT64-backed DATETIMEV2, so min_value != max_value would then reject every multi-value DATETIMEV2 file, disabling pruning for the common case. The INT96 min == max rule and the general DATETIMEV2 path need to be separated.

Both problems are on the v1 path only (enable_file_scanner_v2 defaults true, so v2 is the default reader), and both are pre-existing. They surface for any predicate pushed to a v1 Parquet TIMESTAMP column, including the single-slot expression path and the column-vs-column path proposed in #67774.

What You Expected?

A converted timestamp range that is not monotonic over a clock change, and a legacy INT96 range that PARQUET-1065 marks unreliable, are treated as unusable statistics rather than driving pruning. Pruning and row-level evaluation agree.

How to Reproduce?

Query a v1 Parquet table (set enable_file_scanner_v2 = false) whose TIMESTAMP column is adjusted to UTC, under a session time zone with DST, with a row group whose UTC min/max straddle a fall-back transition, and a predicate selecting a civil time inside the collapsed range. Compare results with and without predicate pushdown. Not reproduced end to end here; this is from the read path and the contrast with the v2 fence.

Anything Else?

The v2 helpers timestamp_min_max_is_safe / utc_timestamp_range_is_monotonic are the reference for the fence to add to v1. Leaving the slot's zone map unset when the range is unusable lets every consumer (single-slot expression, column-vs-column, and any other zone-map reader) fall back conservatively without a per-consumer change.

Are you willing to submit PR?

• Yes I am willing to submit a PR!

Code of Conduct

• I agree to follow this project's Code of Conduct

[Doris-Breakwater]

Breakwater-GitHub-Analysis-Slot: slot_d74038b87ed5

Initial triage

Assessment: confirmed v1 correctness issue, with one scope qualification for INT96. I reviewed the exact reported revision dea1b99e528. This can produce false-negative pruning, so it should be treated as a correctness bug even though enable_file_scanner_v2 defaults to true. At triage time the issue is open, has no labels or comments, and the reporter is willing to submit a PR.

Verified from the code

1. The DST rollback case is valid when an adjusted-to-UTC INT64 timestamp is exposed as DATETIMEV2.

   • ParquetPredicate::parse_min_max_value passes the session cctz::time_zone to PhysicalToLogicalConverter and converts the two physical bounds independently.
   • For an INT64 Parquet TIMESTAMP with isAdjustedToUTC=true, Int64ToTimestamp applies the offset at each instant. There is no v1 check equivalent to timestamp_min_max_is_safe / utc_timestamp_range_is_monotonic before the converted fields become pruning bounds.
   • Therefore, across the New York 2021 rollback, UTC [1636263000, 1636266600] (05:30-06:30 UTC) converts to civil [01:30, 01:30], although an interior value such as 05:59 UTC converts to 01:59. A dt > '2021-11-07 01:45:00' predicate can consequently reject the zone while a matching row exists.
   • The unsafe fields are consumed by both footer row-group pruning (_process_column_stat_filter and _process_expr_zonemap_filter) and page-index pruning (the comparison/in-list predicate path and _process_expr_zonemap_page_filter). All of these paths already fall back conservatively when parsing returns a non-OK status, so rejecting the statistics centrally is the right shape of fix.

2. The max_field typo and over-broad condition are both real.

   • Lines 344-345 read min_field twice, so the local min_value != max_value check cannot fail.
   • The condition is physical INT96 || logical DATETIMEV2. Merely changing the second read to max_field would incorrectly reject every non-singleton INT64-backed DATETIMEV2 range.

3. INT96 impact is narrower than the issue currently states.

   • Footer row-group statistics already pass through read_column_stats, which rejects unequal INT96 min_value/max_value at lines 379-386; the legacy min/max path also classifies INT96 order as unknown and rejects unequal bounds. Thus I do not see a current footer row-group hole for unequal INT96 statistics.
   • Page-index statistics do bypass read_column_stats: _process_page_index_filter copies ColumnIndex.min_values / max_values directly and later calls parse_min_max_value. The dead check therefore leaves the page-index paths exposed. Any claim that unequal INT96 footer statistics prune a row group needs a concrete counterexample to the earlier guard.

The DST risk applies when UTC timestamps are mapped to local DATETIMEV2 (the default when timestamp-TZ mapping is disabled). When enable_mapping_timestamp_tz maps them to TIMESTAMPTZ, the converter preserves UTC ordering and the rollback fence should not disable those bounds, matching the v2 policy.

Recommended change

• In the central v1 statistics parser, validate the raw INT64 timestamp range before converting it: for TIMESTAMP(isAdjustedToUTC=true) mapped to DATETIMEV2 with a session time zone, reject the range if the UTC-to-civil mapping contains a backward transition. Reuse or relocate the existing v2 helper rather than duplicating transition logic; keep TIMESTAMPTZ exempt.
• Separate the INT96 rule from DATETIMEV2. Apply the singleton-only rule only when the physical Parquet type is INT96, preferably by comparing the encoded physical bounds before conversion. General INT64-backed DATETIMEV2 min/max must remain usable.
• Preserve the existing behavior in every caller: an unusable range means no zone map / no pruning, not a scan error.

Regression coverage requested

• A v1 end-to-end case with enable_file_scanner_v2=false, time_zone='America/New_York', an adjusted-to-UTC INT64 TIMESTAMP range spanning the 2021 rollback, and dt > '2021-11-07 01:45:00'; compare results with metadata pruning disabled. Cover both footer row-group statistics and page indexes.
• Unit cases showing rollback ranges are rejected, a transition-free range remains usable, and spring-forward does not get rejected solely for crossing the gap.
• INT96 unequal bounds are unusable on the direct page-index path; equal bounds remain usable; unequal INT64 DATETIMEV2 bounds remain usable.
• If timestamp-TZ mapping is supported on v1, pin that TIMESTAMPTZ ranges keep UTC ordering and are not unnecessarily discarded.

Missing runtime evidence

The code mechanism is sufficient to confirm the defect, but the report does not include an end-to-end reproducer. For a runtime confirmation, please attach either a minimal Parquet file or a deterministic generator plus the table DDL, full SQL/session settings, and parquet-tools metadata showing the logical timestamp annotation, isAdjustedToUTC, physical min/max, column order, and whether page indexes are present. Also include result counts with metadata pruning enabled and disabled; a query profile identifying row-group/page-index pruning would make the affected consumer explicit.

[Doris-Breakwater]

Breakwater-GitHub-Analysis-Slot: slot_d74038b87ed5

Initial triage

Assessment: confirmed v1 correctness issue, with one scope qualification for INT96. I reviewed the exact reported revision dea1b99e528. This can produce false-negative pruning, so it should be treated as a correctness bug even though enable_file_scanner_v2 defaults to true. At triage time the issue is open, has no labels or comments, and the reporter is willing to submit a PR.

Verified from the code

1. The DST rollback case is valid when an adjusted-to-UTC INT64 timestamp is exposed as DATETIMEV2.

   • ParquetPredicate::parse_min_max_value passes the session cctz::time_zone to PhysicalToLogicalConverter and converts the two physical bounds independently.
   • For an INT64 Parquet TIMESTAMP with isAdjustedToUTC=true, Int64ToTimestamp applies the offset at each instant. There is no v1 check equivalent to timestamp_min_max_is_safe / utc_timestamp_range_is_monotonic before the converted fields become pruning bounds.
   • Therefore, across the New York 2021 rollback, UTC [1636263000, 1636266600] (05:30-06:30 UTC) converts to civil [01:30, 01:30], although an interior value such as 05:59 UTC converts to 01:59. A dt > '2021-11-07 01:45:00' predicate can consequently reject the zone while a matching row exists.
   • The unsafe fields are consumed by both footer row-group pruning (_process_column_stat_filter and _process_expr_zonemap_filter) and page-index pruning (the comparison/in-list predicate path and _process_expr_zonemap_page_filter). All of these paths already fall back conservatively when parsing returns a non-OK status, so rejecting the statistics centrally is the right shape of fix.

2. The max_field typo and over-broad condition are both real.

   • Lines 344-345 read min_field twice, so the local min_value != max_value check cannot fail.
   • The condition is physical INT96 || logical DATETIMEV2. Merely changing the second read to max_field would incorrectly reject every non-singleton INT64-backed DATETIMEV2 range.

3. INT96 impact is narrower than the issue currently states.

   • Footer row-group statistics already pass through read_column_stats, which rejects unequal INT96 min_value/max_value at lines 379-386; the legacy min/max path also classifies INT96 order as unknown and rejects unequal bounds. Thus I do not see a current footer row-group hole for unequal INT96 statistics.
   • Page-index statistics do bypass read_column_stats: _process_page_index_filter copies ColumnIndex.min_values / max_values directly and later calls parse_min_max_value. The dead check therefore leaves the page-index paths exposed. Any claim that unequal INT96 footer statistics prune a row group needs a concrete counterexample to the earlier guard.

The DST risk applies when UTC timestamps are mapped to local DATETIMEV2 (the default when timestamp-TZ mapping is disabled). When enable_mapping_timestamp_tz maps them to TIMESTAMPTZ, the converter preserves UTC ordering and the rollback fence should not disable those bounds, matching the v2 policy.

Recommended change

• In the central v1 statistics parser, validate the raw INT64 timestamp range before converting it: for TIMESTAMP(isAdjustedToUTC=true) mapped to DATETIMEV2 with a session time zone, reject the range if the UTC-to-civil mapping contains a backward transition. Apply the same instant-semantics handling to legacy TIMESTAMP_MILLIS / TIMESTAMP_MICROS annotations. Reuse or relocate the existing v2 helper rather than duplicating transition logic; keep TIMESTAMPTZ exempt.
• Separate the INT96 rule from DATETIMEV2. Apply the singleton-only rule only when the physical Parquet type is INT96, preferably by comparing the encoded physical bounds before conversion. General INT64-backed DATETIMEV2 min/max must remain usable.
• Preserve the existing behavior in every caller: an unusable range means no zone map / no pruning, not a scan error.

Regression coverage requested

• A v1 end-to-end case with enable_file_scanner_v2=false, time_zone='America/New_York', an adjusted-to-UTC INT64 TIMESTAMP range spanning the 2021 rollback, and dt > '2021-11-07 01:45:00'; compare results with metadata pruning disabled. Cover both footer row-group statistics and page indexes.
• Unit cases showing rollback ranges are rejected, a transition-free range remains usable, and spring-forward does not get rejected solely for crossing the gap.
• INT96 unequal bounds are unusable on the direct page-index path; equal bounds remain usable; unequal INT64 DATETIMEV2 bounds remain usable.
• If timestamp-TZ mapping is supported on v1, pin that TIMESTAMPTZ ranges keep UTC ordering and are not unnecessarily discarded.

Missing runtime evidence

The code mechanism is sufficient to confirm the defect, but the report does not include an end-to-end reproducer. For a runtime confirmation, please attach either a minimal Parquet file or a deterministic generator plus the table DDL, full SQL/session settings, and a metadata dump showing the writer, logical timestamp annotation, isAdjustedToUTC, physical min/max, column order, and whether page indexes are present. For the INT96 claim in particular, include a file with an unequal ColumnIndex range so the page-index reachability is explicit. Also include result counts with metadata pruning enabled and disabled; profile counters such as RowGroupsFilteredByMinMax, ParquetExprZoneMapFilteredRowGroups, FilteredRowsByPage, and PageIndexReadCalls would identify the affected consumer.

Temporary mitigation

Prefer the default v2 scanner. If the v1 scanner is required before a fix is available, set enable_parquet_filter_by_min_max = false disables these min/max pruning paths at the cost of additional I/O and decoding.

[LuciferYang]

Opened #68148.