[fix](function) Preserve subsecond precision in convert_tz DST gaps #67493
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,6 +44,8 @@ | |
| import java.time.Instant; | ||
| import java.time.LocalDateTime; | ||
| import java.time.ZoneId; | ||
| import java.time.zone.ZoneOffsetTransition; | ||
| import java.time.zone.ZoneRules; | ||
| import java.util.List; | ||
|
|
||
| /** | ||
|
|
@@ -107,11 +109,8 @@ public boolean isMonotonic(Literal lower, Literal upper) { | |
| if (fromZone == null || toZone == null) { | ||
| return false; | ||
| } | ||
| if (lower == null || upper == null) { | ||
| return toZone.getRules().isFixedOffset() && !mayHaveFractionalSecondSourceGap(fromZone); | ||
| } | ||
| LocalDateTime lowerDateTime = toLocalDateTime(lower); | ||
| LocalDateTime upperDateTime = toLocalDateTime(upper); | ||
|
|
@@ -124,15 +123,24 @@ public boolean isMonotonic(Literal lower, Literal upper) { | |
| if (upperDateTime.isBefore(lowerDateTime)) { | ||
| return false; | ||
| } | ||
| if (mayHaveFractionalSecondSourceGap(fromZone) | ||
|
Contributor
There was a problem hiding this comment. [P1] Avoid FE-only timezone rules when certifying source-gap monotonicity
FE derives an image around |
||
| && hasGapResetInRange(fromZone, lowerDateTime, upperDateTime)) { | ||
| return false; | ||
| } | ||
| if (toZone.getRules().isFixedOffset()) { | ||
| return true; | ||
| } | ||
| /* | ||
| * convert_tz can be treated as a composition of two mappings: | ||
| * | ||
| * source local time x -> instant by from_tz -> target local time by to_tz. | ||
| * | ||
| * For whole-second values, the first mapping is monotonic non-decreasing. A spring gap in | ||
| * from_tz flattens skipped local times to the transition instant, and a fall-back overlap | ||
| * uses the pre-transition offset before jumping forward at the overlap end. For fractional | ||
| * values, each skipped civil second preserves its fraction, so crossing an integer-second | ||
| * boundary inside the gap resets that fraction and makes the mapping non-monotonic. That | ||
| * case is rejected above. | ||
| * | ||
| * The second mapping, instant -> to_tz local time, is also monotonic non-decreasing except | ||
| * at a to_tz fall-back transition, where the displayed local time jumps backward. Therefore | ||
|
|
@@ -150,6 +158,34 @@ public boolean isMonotonic(Literal lower, Literal upper) { | |
| return !DateUtils.hasFallbackTransitionInInstantRange(toZone, lowerInstant, upperInstant); | ||
| } | ||
|
|
||
| private boolean mayHaveFractionalSecondSourceGap(ZoneId fromZone) { | ||
| return child(0).getDataType() instanceof DateTimeV2Type | ||
| && ((DateTimeV2Type) child(0).getDataType()).getScale() > 0 | ||
| && !fromZone.getRules().isFixedOffset(); | ||
| } | ||
|
|
||
| private boolean hasGapResetInRange(ZoneId fromZone, LocalDateTime lower, LocalDateTime upper) { | ||
| ZoneRules rules = fromZone.getRules(); | ||
| Instant lowerInstant = DateTimeLiteral.convertLocalToInstant(lower, fromZone); | ||
| ZoneOffsetTransition transition = rules.getTransition(lower); | ||
| if (transition == null) { | ||
| transition = rules.nextTransition(lowerInstant.minusNanos(1)); | ||
| } | ||
| while (transition != null && !transition.getDateTimeBefore().isAfter(upper)) { | ||
| if (transition.isGap()) { | ||
| LocalDateTime firstReset = transition.getDateTimeBefore().plusSeconds(1); | ||
| LocalDateTime lastReset = transition.getDateTimeAfter(); | ||
| LocalDateTime nextReset = lower.isBefore(firstReset) | ||
| ? firstReset : lower.withNano(0).plusSeconds(1); | ||
| if (!nextReset.isAfter(upper) && !nextReset.isAfter(lastReset)) { | ||
| return true; | ||
| } | ||
| } | ||
| transition = rules.nextTransition(transition.getInstant()); | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isPositive() { | ||
| return true; | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] Preserve old-FE pruning semantics during BE-first upgrades
Doris upgrades BEs before FEs, so this unconditional result change runs while an old FE can still certify every fixed-target
convert_tzas monotonic and fold source-gap endpoints with the old zero-fraction policy. With the same Europe/Paris rules on both sides, an old FE can project aDATETIMEV2(6)partition[2021-03-28 02:00:00, 2021-03-28 03:00:00)to the singleton01:00:00and prune it forconvert_tz(ts,'Europe/Paris','UTC') > '2021-03-28 01:00:00.500000'. This new BE path maps a stored02:30:00.900000to01:00:00.900000, so that pruned row actually matches.Please stage this semantic change behind an old-FE/new-BE compatibility contract (for example, a query capability or versioned function implementation), and add a mixed-version pruning regression, so the supported rolling-upgrade window cannot drop rows.