[fix](be) Prevent DST timestamp statistics mispruning#65497
Conversation
### What problem does this PR solve? Issue Number: N/A Related PR: apache#64456 Problem Summary: FileScannerV2 converts UTC timestamp statistics into session-local DATETIMEV2 values before ZoneMap evaluation. Across a daylight-saving time rollback, that conversion is not monotonic, so converted metadata endpoints may exclude local values that occur inside the UTC interval. Parquet row-group and page-index pruning, and ORC statistics pruning, could therefore skip matching data. Detect backward timezone transitions within UTC statistics ranges and conservatively disable min/max pruning for those ranges while retaining statistics for monotonic ranges and TIMESTAMPTZ mappings. ### Release note Fix incorrect Parquet and ORC file scan pruning for UTC timestamps across daylight-saving time rollbacks. ### Check List (For Author) - Test: Unit Test - Remote ASAN BE UT: TimestampStatisticsTest.*, ParquetStatisticsTransformTest.*, and NewOrcReaderTest.AggregatePushdownTimestamp* (7 passed) - Behavior changed: Yes. Non-monotonic local timestamp statistics no longer prune rows or pages. - Does this need documentation: No
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated review result: request changes.
Critical checkpoint conclusions:
- Goal and tests: the PR addresses DST rollback mispruning for converted timestamp statistics and adds focused BE unit tests, but it misses the ORC scan-time SARG pruning path and the Parquet TIMESTAMPTZ mapping/config path.
- Scope: the change is small and focused, but the implementation does not cover all parallel timestamp statistics consumers.
- Concurrency/lifecycle/config: no new concurrency or lifecycle risk found. The existing
enable_mapping_timestamp_tzconfiguration path is affected by the Parquet guard. - Compatibility/persistence/data writes: no storage format, persistence, FE/BE protocol, or write-transaction changes found.
- Parallel paths: ORC aggregate min/max is guarded, but normal ORC SearchArgument stripe pruning is not; Parquet DATETIMEV2 is guarded, but TIMESTAMPTZ mapping should keep safe UTC min/max.
- Test coverage: helper and Parquet DATETIMEV2 rollback coverage were added. Missing coverage remains for ORC rollback scan pruning and Parquet
enable_mapping_timestamp_tzstatistics. I could not run local BE UTs because this checkout lacksthirdparty/installedandthirdparty/installed/bin/protoc. - Performance: the Parquet guard unnecessarily disables safe pruning and min/max aggregate pushdown for TIMESTAMPTZ-mapped UTC timestamps.
User focus: no additional user-provided focus points.
Subagent conclusions: FS-1 became inline comment M-1; OR-1 became inline comment M-2; TSC-1 and TSC-2 were duplicates of those accepted candidates. Convergence round 1 ended with optimizer-rewrite, tests-session-config, and format-statistics all reporting NO_NEW_VALUABLE_FINDINGS for the same final ledger/comment set.
| timestamp_statistics->getMaximum(), timestamp_statistics->getMaximumNanos())); | ||
| return true; | ||
| } | ||
| if (!format::utc_timestamp_range_is_monotonic( |
There was a problem hiding this comment.
This only disables the ZoneMap conversion path, but ORC scan pruning still builds timestamp SearchArguments in _init_search_argument_from_local_filters() and uses getNeedReadStripes() before row-level filters run. For a TIMESTAMP_INSTANT column decoded as session-local DATETIMEV2, a rollback value such as 2021-11-07 01:30 in America/New_York names two UTC instants; the SARG literal conversion picks a single timestamp and lets ORC compare it to stripe min/max, so a stripe containing the other repeated-hour instant can still be pruned. Please also guard or disable timestamp SARG pruning for non-monotonic local mappings, and add an ORC scan test for that rollback case.
|
|
||
| bool timestamp_min_max_is_safe(const ParquetColumnSchema& column_schema, int64_t min_value, | ||
| int64_t max_value, const cctz::time_zone* timezone) { | ||
| if (!column_schema.type_descriptor.is_timestamp || |
There was a problem hiding this comment.
This also disables min/max for adjusted-to-UTC Parquet timestamps after enable_mapping_timestamp_tz maps the column to TYPE_TIMESTAMPTZ. In that mode the decoded values are TimestampTzValues stored and compared in UTC, so the local DATETIMEV2 monotonicity problem does not apply; ORC handles the analogous path by returning TYPE_TIMESTAMPTZ min/max before running this guard. Since apply_timestamp_tz_mapping() leaves the Parquet timestamp flags set, rollback-spanning row groups/pages lose safe pruning and Parquet min/max aggregate pushdown returns NotSupported. Please skip this guard when the effective Doris type is TYPE_TIMESTAMPTZ, and add coverage for that config path.
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Disable unsafe ORC timestamp SARG pushdown in civil years with backward timezone transitions, while preserving Parquet min/max pruning for TIMESTAMPTZ values that retain UTC ordering. Add ORC, Parquet, and timezone transition regression coverage.
|
run buildall |
|
/review |
There was a problem hiding this comment.
Static review completed for current head 833fbdf. I did not find any additional non-duplicate blocking issue beyond the two existing inline threads, and the current patch appears to address both of those areas.
Critical checkpoint conclusions:
- Goal and tests: The patch targets unsafe timestamp statistics/SARG pruning across DST rollback and adds focused coverage for ORC repeated-civil-time SARG behavior, Parquet adjusted-to-UTC DATETIMEV2 statistics, TIMESTAMPTZ mapped statistics, and shared transition/flooring helpers.
- Scope and clarity: The change is limited to timestamp statistics/SARG safety and nearby tests.
- Concurrency, lifecycle, persistence, transactionality, FE/BE protocol, and configuration: Not involved.
- Parallel paths: ORC ZoneMap/aggregate min-max, ORC SearchArgument pruning, Parquet row-group statistics, Parquet page-index statistics, and Parquet aggregate min-max were checked. Unsafe local DATETIMEV2 statistics fail open; TIMESTAMPTZ preserves UTC ordering.
- Error/fail-open behavior: Unsafe statistics clear min/max or return NotSupported instead of pruning; unsafe ORC TIMESTAMP_INSTANT SARG literals become non-buildable and row-level filters remain authoritative.
- Test coverage: The main changed runtime paths are covered by new BE unit tests. A dedicated Parquet page-index runtime fixture is not added, but the page-index path shares the same safety helper and fails open when it cannot build trustworthy statistics.
- User focus: No additional user-provided focus was present.
- Subagent convergence: NORMAL-ORC, NORMAL-PARQUET, RISK-STATS, and RISK-SARG all returned NO_NEW_VALUABLE_FINDINGS in round 1 after reading the current ledger.
Validation was static only. Per the review prompt I did not build or run tests; this checkout also lacks .worktree_initialized, thirdparty/installed, and thirdparty/installed/bin/protoc.
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
|
Superseded by #65500, which consolidates and validates the four related fixes. |
2 participants
What problem does this PR solve?
Issue Number: N/A
Related PR: #64456
Problem Summary: FileScannerV2 converts UTC timestamp statistics into session-local DATETIMEV2 values before ZoneMap evaluation. Across a daylight-saving time rollback, that conversion is not monotonic, so converted metadata endpoints may exclude local values that occur inside the UTC interval. Parquet row-group and page-index pruning, and ORC statistics pruning, could therefore skip matching data. Detect backward timezone transitions within UTC statistics ranges and conservatively disable min/max pruning for those ranges while retaining statistics for monotonic ranges and TIMESTAMPTZ mappings.
Release note
Fix incorrect Parquet and ORC file scan pruning for UTC timestamps across daylight-saving time rollbacks.
Check List (For Author)
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)