[fix](nereids) Fix DST spring-forward gap handling in timestamptz literal#62945
[fix](nereids) Fix DST spring-forward gap handling in timestamptz literal#62945zclllyybb merged 1 commit intoapache:masterfrom
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
/review |
There was a problem hiding this comment.
Pull request overview
Fixes incorrect TIMESTAMPTZ literal evaluation when the specified local timestamp falls into a DST “spring-forward” gap (e.g., America/New_York at 2024-03-10 02:30:00), ensuring the literal is resolved to the correct instant instead of effectively ignoring the gap.
Changes:
- Replaced manual offset-difference adjustment logic in
DateTimeLiteralwith aLocalDateTime.atZone(...).withZoneSameInstant(...)conversion to correctly resolve DST gaps. - Added a regression test suite + expected output covering named-zone, explicit-offset, and session-timezone (implicit) DST-gap literals and verifying equality.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| regression-test/suites/datatype_p0/timestamptz/test_timestamptz_dst_gap.groovy | New regression coverage for DST spring-forward gap behavior for TIMESTAMPTZ literals. |
| regression-test/data/datatype_p0/timestamptz/test_timestamptz_dst_gap.out | Expected results for the new DST-gap test queries. |
| fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/DateTimeLiteral.java | Uses zone-aware conversion via withZoneSameInstant to correctly handle DST gaps during literal parsing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
run buildall |
|
run buildall |
|
/review |
There was a problem hiding this comment.
No blocking issues found in this review.
Critical checkpoint conclusions:
- Goal/test coverage: The PR targets DST spring-forward gap handling for timestamptz/date-time literals. The implementation uses Java timezone resolution via
LocalDateTime.atZone(...).withZoneSameInstant(...), and the new regression test covers explicit named-zone gap, explicit offsets around the gap, and implicit session-zone gap handling. - Scope/focus: The change is small and focused on Nereids date/time literal timezone normalization plus one range guard.
- Concurrency/lifecycle/config: No concurrency, special lifecycle, or configuration changes are introduced.
- Compatibility/parallel paths: The reviewed string-literal cast and constant-folding paths continue to normalize explicit zones consistently; implicit TIMESTAMPTZ session-zone conversion already uses the same Java timezone conversion path through
convert_tz. - Error handling/data correctness: Range validation still runs after timezone conversion, and adding
year < 0prevents converted values before year 0000 from being silently accepted. - Tests/results: The regression test follows the drop-before-use pattern and orders table output. I did not run the regression test in this review environment.
- Observability/performance: No additional observability is needed for this literal parsing fix; no material performance issue found.
User focus: No additional user-provided review focus was specified.
|
run buildall |
1 similar comment
|
run buildall |
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
…eral (#62945) fix wrong timestamptz result when cast through DST. before may ignore the DST.
…eral (#62945) fix wrong timestamptz result when cast through DST. before may ignore the DST.
fix wrong timestamptz result when cast through DST.
before may ignore the DST.