Doris-Breakwater commented on issue #66120: URL: https://github.com/apache/doris/issues/66120#issuecomment-5091283531
## Initial maintainer assessment **Verdict: confirmed FE/Nereids overload-resolution correctness bug at `f499c78c67c`.** The supplied commit, deterministic SQL, expected/actual results, and `EXPLAIN VERBOSE` output are sufficient for triage; no additional log or Doris profile is needed to establish the `DATEDIFF` defect. Because this can silently change query results, it should be treated as a high-priority correctness issue rather than a cosmetic binding difference. The issue currently has no labels, assignee, or milestone. `area/nereids` and `area/sql/function` are appropriate existing routing labels. ### Code-level confirmation - `ExplicitlyCastableSignature.searchSignature()` reaches the explicit-cast round for non-literal `VARCHAR` slots. In `SearchSignature.doMatchTypes()`, timezone scoring is performed only when `ExpressionUtils.getLiteralAfterUnwrapNullable(argument)` returns a literal. A `SlotReference` therefore receives neither the `TIMESTAMPTZ` penalty for zone-less text nor a preference for `DATETIMEV2`. - For two `VARCHAR` slots, the relevant overloads have the same non-strict-match count and timezone score. `SearchSignature.result()` retains the first equal candidate, and all 12 listed `*Diff.java` classes place the `TIMESTAMPTZ` overload before `DATETIMEV2`. - `cast_to_timestamptz_impl.hpp` confirms that zone-less text is interpreted in the session timezone and converted to UTC, while `TimestampTzValue::daynr()` reads the stored UTC datetime. This directly explains the reported `DATEDIFF` result: under `+08:00`, only the midnight operand crosses into the previous UTC date, reducing the day-number difference by one. - This is not a folding-only path. Once the FE inserts `CAST(... AS TIMESTAMPTZ(6))`, the BE cast and `TYPE_TIMESTAMPTZ` diff implementation operate on UTC values, so `debug_skip_fold_constant` cannot repair the slot case. The same signature-order defect is present on the current `branch-4.0` and `branch-4.1` heads inspected during triage. One version-detail correction is needed: the current `branch-4.1` contains the #64127 backport (`17bc4a19f9d`), but current `branch-4.0` (`6ffd9b19a168`) does not contain that change or an equivalent `getLiteralAfterUnwrapNullable` implementation. The slot bug exists on both branches, but 4.0 also retains the earlier nullable-literal binding gap. ### Scope qualification All 12 functions are incorrectly *bound* for an all-`VARCHAR` slot call, but “every function produces a wrong numeric result whenever the session timezone is non-UTC” is broader than the code supports. - With a fixed offset such as `+08:00`, subtracting the same offset from both operands cancels for elapsed-time calculations such as `TIME_DIFF`, `HOURS_DIFF`, `MINUTES_DIFF`, `SECONDS_DIFF`, `MILLISECONDS_DIFF`, and `MICROSECONDS_DIFF`; their values can remain numerically identical despite the wrong overload. - `DATEDIFF` exposes the bug when the UTC conversion changes the calendar date of one operand but not the other. Calendar-unit functions can likewise differ for boundary-sensitive inputs. - In a named timezone with a DST/offset transition between the operands, the offsets need not cancel, so elapsed-time units can also diverge from the wall-clock `DATETIMEV2` result. The PR description and tests should therefore say that all 12 functions are susceptible to wrong semantics through mis-binding, while the observable numeric error depends on the unit, input values, and timezone transition. ### Fix assessment Moving the `TIMESTAMPTZ` overload after `DATETIMEV2` is a reasonable focused fix for these functions: - exact `TIMESTAMPTZ`, `DATETIMEV2`, and `DATEV2` arguments are selected in an earlier identical/null-identical round, so list order does not change those cases; - a timezone-bearing string literal still receives the positive `TIMESTAMPTZ` score; - a zone-less literal still receives the negative `TIMESTAMPTZ` score; - an uninspectable `VARCHAR` slot then falls back to the legacy wall-clock `DATETIMEV2` interpretation. Before merging, maintainers should explicitly decide the contract for timezone-bearing text stored in a `VARCHAR` column. The FE cannot inspect row values during binding, so the reorder will also send values such as `'2025-08-01 00:00:00+08:00'` from a `VARCHAR` slot to `DATETIMEV2`. If such values are intended to have instant semantics, callers should be required to use a typed `TIMESTAMPTZ` column or an explicit cast, and that behavior should be documented and tested. If automatic per-row timezone detection is required, signature reordering alone cannot implement it. ### Recommended next steps 1. Accept the reporter's offer of a focused PR for the 12 signature lists, with separate backports for supported branches after master validation. 2. Add FE signature/plan assertions for a physical `VARCHAR` column, subquery slot, and `UNION ALL` slot. Assert `DATETIMEV2` in `EXPLAIN`, not only numeric results, because fixed-offset elapsed-unit results can hide the wrong overload. 3. Add non-UTC execution regressions for boundary-sensitive `DATEDIFF`/calendar cases and a named-zone DST case for elapsed units. Run representative cases with constant folding enabled and disabled. 4. Preserve coverage for timezone-bearing literals, explicitly cast `TIMESTAMPTZ` values, typed `TIMESTAMPTZ` columns, `DATETIMEV2` columns, and `DATEV2` columns. 5. Add a focused `SearchSignature` unit test for the equal-score non-literal string case. A broader change to generic timezone scoring should be considered only after auditing other functions with both `TIMESTAMPTZ` and `DATETIMEV2` overloads; the 12-function reorder is the lower-risk immediate correction. 6. Correct the branch/#64127 statement and narrow the “all functions always wrong” wording as described above. Neither correction blocks confirmation of the reported `DATEDIFF` bug. Breakwater-GitHub-Analysis-Slot: slot_3be1338239f5 -- 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]
