Baymine opened a new issue, #66120:
URL: https://github.com/apache/doris/issues/66120

   ## Search before asking
   
   - [x] I had searched in the 
[issues](https://github.com/apache/doris/issues?q=is%3Aissue) and found no 
similar issues.
   
   ## Version
   
   apache/doris master at `f499c78c67c` (also affects `branch-4.0`, 
`branch-4.1`). Related previous fix `43800f3bf51` (PR #64127) is included, but 
is **insufficient** for the case described below.
   
   ## What's Wrong?
   
   `DATEDIFF` (and every other `*_diff` date function) silently returns a wrong 
result whenever:
   
   1. its argument is a **string-typed** value that is not a bare literal — 
e.g. a `varchar` column, a subquery projection slot, or a UNION-ALL output 
slot, **and**
   2. the session `time_zone` is not `Etc/UTC`.
   
   Under those two conditions Nereids binds the call to the `TIMESTAMPTZ` 
overload, which routes the value through `varchar → CAST(timestamptz(6))` and 
evaluates `daynr()` on the internal UTC-shifted representation, producing an 
off-by-one (or off-by-N) result.
   
   The reason this has not been noticed in CI: master's default session 
`time_zone` is `Etc/UTC`, under which the varchar→timestamptz cast is a no-op 
shift and the wrong overload silently produces the right answer. **Any non-UTC 
deployment reproduces the bug immediately.**
   
   ### Reproducer against apache/master at `f499c78c67c`
   
   ```sql
   -- master session default is Etc/UTC → looks fine
   SELECT DATEDIFF(a, b)
   FROM (SELECT '2025-08-01 00:00:00' AS a, '2025-05-30 11:40:09' AS b) t;
   -- returns 63  ✓
   
   -- flip to any non-UTC session tz (e.g. any China / East Asia deployment)
   SET time_zone='+08:00';
   SELECT DATEDIFF(a, b)
   FROM (SELECT '2025-08-01 00:00:00' AS a, '2025-05-30 11:40:09' AS b) t;
   -- returns 62  ✗  (off-by-one, WRONG)
   
   -- Same bug on a real varchar column
   CREATE TABLE td (a varchar(30), b varchar(30)) 
PROPERTIES('replication_num'='1');
   INSERT INTO td VALUES ('2025-08-01 00:00:00', '2025-05-30 11:40:09');
   SET time_zone='+08:00';
   SELECT DATEDIFF(a, b) FROM td;
   -- returns 62  ✗
   
   -- Same bug in the shape that first surfaced this
   SET time_zone='+08:00';
   SELECT DATEDIFF(a, b)
   FROM (
     SELECT '2025-08-01 00:00:00' AS a, '2025-05-30 11:40:09' AS b
     UNION ALL SELECT '2025-08-02 00:00:00', '2024-09-30 19:40:02'
   ) t;
   -- returns 62 / 305  ✗  (expected 63 / 306)
   ```
   
   `EXPLAIN VERBOSE` on any of the failing queries shows:
   
   ```
   datediff(cast(a as TIMESTAMPTZ(6)), cast(b as TIMESTAMPTZ(6)))
   ```
   
   ## What You Expected?
   
   `DATEDIFF(a, b)` should return the wall-clock day difference of the two 
input strings, **independent of session `time_zone`**. Expected values above: 
`63` and `63 / 306`.
   
   ## How to Reproduce?
   
   1. Deploy any FE at master `f499c78c67c` (or `branch-4.0` / `branch-4.1`).
   2. Connect via MySQL client.
   3. Run the SQL statements above.
   
   Reproduces deterministically on every non-UTC `time_zone`, tested on 
`Asia/Shanghai`, `+08:00`.
   
   ## Root Cause
   
   Two independent facts combine into the bug:
   
   ### 1. Signature-order tie-break routes varchar to `TIMESTAMPTZ`
   
   
`fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/DateDiff.java`:
   
   ```java
   public static final List<FunctionSignature> SIGNATURES = ImmutableList.of(
       FunctionSignature.ret(IntegerType.INSTANCE)
               .args(TimeStampTzType.WILDCARD, TimeStampTzType.WILDCARD),     
// <- listed FIRST
       FunctionSignature.ret(IntegerType.INSTANCE)
               .args(DateTimeV2Type.WILDCARD, DateTimeV2Type.WILDCARD),
       FunctionSignature.ret(IntegerType.INSTANCE)
               .args(DateV2Type.INSTANCE, DateV2Type.INSTANCE));
   ```
   
   `SearchSignature.doMatchTypes` only fires the `timeZoneCoersionScore--` 
penalty on `TIMESTAMPTZ` when the argument is a **literal it can inspect** — 
via `ExpressionUtils.getLiteralAfterUnwrapNullable(argument)` added by #64127. 
For a `SlotReference` typed `varchar` (which is what every subquery projection 
slot, UNION-ALL output slot, and real varchar column becomes at binding time), 
that helper returns `Optional.empty()`, so no scoring penalty fires. All three 
signatures then tie on `nonStrictMatchedWithoutStringLiteralCoercion` and the 
tie-break in `SearchSignature.result()` (lines 115–142) picks whichever 
candidate is seen first — **TIMESTAMPTZ**.
   
   **#64127 only fixed the bare-literal / `Nullable(Literal)` subcase.** The 
much broader slot / column subcase is still latent.
   
   ### 2. `varchar → timestamptz(6)` cast on non-UTC session shifts the value
   
   Per `be/src/exprs/function/cast/cast_to_timestamptz_impl.hpp`:
   
   > When timezone info is absent, `TIMESTAMP_TZ` treats input as local time 
and converts to UTC
   
   And `TimestampTzValue::daynr()` returns `_utc_dt.daynr()` — the daynr of the 
shifted UTC value. So `'2025-08-01 00:00:00'` under session `+08:00` becomes 
UTC `2025-07-31 16:00:00`, and its `daynr()` is one less than the wall-clock 
date.
   
   The signature is picked at bind time and the shift happens at cast time; 
both paths agree even when `debug_skip_fold_constant=true`, so the bug is not a 
folding artifact.
   
   ## Scope
   
   This same signature ordering exists in **12 date functions**, so all of them 
are affected identically:
   
   ```
   DateDiff, DaysDiff, HoursDiff, MicroSecondsDiff, MilliSecondsDiff,
   MinutesDiff, MonthsDiff, QuartersDiff, SecondsDiff, TimeDiff,
   WeeksDiff, YearsDiff
   ```
   
   ## Suggested Fix
   
   Reorder the `SIGNATURES` list in the 12 `*Diff.java` files to put 
`TIMESTAMPTZ` **last**:
   
   ```java
   public static final List<FunctionSignature> SIGNATURES = ImmutableList.of(
       FunctionSignature.ret(IntegerType.INSTANCE)
               .args(DateTimeV2Type.WILDCARD, DateTimeV2Type.WILDCARD),
       FunctionSignature.ret(IntegerType.INSTANCE)
               .args(DateV2Type.INSTANCE, DateV2Type.INSTANCE),
       FunctionSignature.ret(IntegerType.INSTANCE)
               .args(TimeStampTzType.WILDCARD, TimeStampTzType.WILDCARD));  // 
<- moved LAST
   ```
   
   ### Why this is correct (traced through `SearchSignature`)
   
   - `DATEDIFF(datetimev2_col, datetimev2_col)` — `DateTimeV2` matches 
identically (`nonStrictMatched=0`); still wins. ✓
   - `DATEDIFF(timestamptz_col, timestamptz_col)` — `TIMESTAMPTZ` matches 
identically; still wins even when listed last. ✓
   - `DATEDIFF(date_col, date_col)` — `DateV2` matches identically; still wins. 
✓
   - `DATEDIFF('str+08:00', 'str+08:00')` — literal-with-TZ still routes to 
`TIMESTAMPTZ` because `timeZoneCoersionScore=+1` beats `0` in `doMatchTypes` 
regardless of list order. ✓
   - `DATEDIFF('2025-08-01 00:00:00', '2025-05-30 11:40:09')` — 
literal-without-TZ: `TIMESTAMPTZ` gets `-1`, `DateTimeV2` gets `0`, 
`DateTimeV2` wins. ✓ (matches #64127's intent)
   - `DATEDIFF(varchar_col, varchar_col)` — **the failing case**: all three 
sigs tie; first candidate wins → now `DateTimeV2` → wall-clock semantics → 
correct. ✓
   
   Verified locally against apache/master at `f499c78c67c` with the reorder 
applied to just `DateDiff.java`:
   
   ```sql
   SET time_zone='+08:00'; SELECT DATEDIFF(a,b) FROM td;                  -- 63 
✓
   SET time_zone='Asia/Shanghai'; SELECT DATEDIFF(a,b) FROM td;           -- 63 
✓
   SET time_zone='America/Los_Angeles'; SELECT DATEDIFF(a,b) FROM td;     -- 63 
✓
   SET time_zone='+14:00'; SELECT DATEDIFF(a,b) FROM td;                  -- 63 
✓
   SET time_zone='-12:00'; SELECT DATEDIFF(a,b) FROM td;                  -- 63 
✓
   ```
   
   Happy to send a PR with the 12-file reorder + regression tests covering (a) 
varchar column, (b) subquery slot, (c) UNION-ALL slot, and (d) all shapes under 
both UTC and non-UTC session tz.
   
   ## Anything Else?
   
   - Prior related fix: #64127 (fixes bare-literal and `Nullable(Literal)` 
cases only).
   - Related infrastructure: the `matchType` scoring introduced in #57586, and 
the TIMESTAMPTZ overloads added in #59206.
   - Not caused by folding — reproduces identically with `SET 
debug_skip_fold_constant=true`.
   
   ## Are you willing to submit PR?
   
   - [x] Yes I am willing to submit a PR!
   
   ## Code of Conduct
   
   - [x] I agree to follow this project's [Code of 
Conduct](https://www.apache.org/foundation/policies/conduct)
   


-- 
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]

Reply via email to