github-actions[bot] commented on code in PR #70003:
URL: https://github.com/apache/doris/pull/70003#discussion_r4545571206
##########
be/src/core/value/timestamptz_value.cpp:
##########
@@ -168,7 +197,9 @@ void TimestampTzValue::convert_local_to_utc(const
cctz::time_zone& local_time_zo
cctz::civil_second local_cs(dt.year(), dt.month(), dt.day(), dt.hour(),
dt.minute(),
dt.second());
const auto lookup = local_time_zone.lookup(local_cs);
- cctz::time_point<cctz::seconds> local_tp = cctz::convert(local_cs,
local_time_zone);
+ // Use the pre-transition offset to shift gap input by the gap length,
e.g. New York's
+ // 2024-03-10 02:30 becomes 03:30. The preferred offset applies only to an
overlap.
+ auto local_tp = lookup.pre;
Review Comment:
[P2] Preserve gap clamping for existing floor callers
This preferred-offset overload also serves `date_trunc` and HOUR/MINUTE
floor/ceil, so changing every skipped time to `lookup.pre` changes those
existing functions too. In an `America/New_York` session, take a table-column
`ts = '2024-03-10 03:15:00-04:00'` and evaluate `hour_floor(ts, 1,
CAST('2024-03-09 00:30:00-05:00' AS TIMESTAMPTZ))`. `time_round_two_args`
computes the missing local `02:30`; this now resolves to `03:30-04:00`, later
than `ts`, whereas the previous `cctz::convert` resolves it to the transition
at `03:00-04:00` ([CCTZ
conversion](https://github.com/google/cctz/blob/master/include/cctz/time_zone.h#L235-L244)).
Keep the prior gap policy for existing rounding callers and apply gap-length
shifting specifically to the new DAY arithmetic. This is a result-correctness
regression distinct from the already-addressed duplicate-lookup performance
comment; the added DAY tests do not cover it.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/SimplifyArithmeticComparisonRule.java:
##########
@@ -112,6 +113,23 @@ private static boolean couldRearrange(ComparisonPredicate
cmp) {
return false;
}
+ // The rule can move a date interval only if the date function keeps
the order of input values.
+ // It calls isMonotonic(null, null) because it has no input range
bounds.
+ // Calendar day addition can change UTC time order for TIMESTAMPTZ
values when the UTC offset changes.
+ //
+ // Example in America/New_York:
+ // ts = 2024-11-03 01:50:00-04:00
+ // cutoff = 2024-11-04 01:30:00-05:00
+ // days_add(ts, 1) < cutoff is false.
+ // ts < days_sub(cutoff, 1) is true.
+ // The subtraction selects the second 01:30 on November 3, with UTC
offset -05:00.
+ //
+ // The check keeps the original comparison if isMonotonic returns
false.
+ if (cmp.left() instanceof DateAddSubMonotonic
+ && !((DateAddSubMonotonic) cmp.left()).isMonotonic(null,
null)) {
Review Comment:
[P2] Validate the inverse before rearranging DAY predicates
Fixed-offset sessions still pass this guard even when the newly introduced
inverse fails. For example, in `+08:00`, consider this reduced plan (both
timestamps are valid UTC values):
```text
Filter(days_add(ts, 1) < TIMESTAMPTZ '9999-12-31 23:00:00+00:00')
Scan(ts = TIMESTAMPTZ '9999-12-29 23:00:00+00:00')
```
The original add produces UTC `9999-12-30 23:00:00`, so the row qualifies.
`tryRearrangeChildren` replaces the predicate with `ts < days_sub(cutoff, 1)`,
but the cutoff's session-local year is 10000 and the new
`plusDays`/`date_add_days` source-year check rejects it. Failed FE folding
leaves that inverse expression for BE evaluation, turning a valid query into an
out-of-range error; before this PR the inverse used UTC arithmetic and
succeeded. Preserve the original comparison when the inverse cannot be
evaluated as a valid literal, including fixed-offset sessions, and cover this
boundary in the rewrite tests.
--
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]
