[fix](nereids) Prefer DATETIMEV2 over TIMESTAMPTZ when binding *_diff functions on non-literal string args - #67238
Open
lets-order-some-fries wants to merge 1 commit into
Conversation
… functions on non-literal string args All 12 *_diff scalar functions listed their TIMESTAMPTZ signature first in SIGNATURES. SearchSignature's timezone-coercion penalty only fires for literals it can inspect (ExpressionUtils.getLiteralAfterUnwrapNullable, apache#64127), so for a string-typed non-literal argument (varchar column, subquery projection slot, UNION-ALL output slot) every candidate signature ties and the tie-break keeps the first-listed candidate: TIMESTAMPTZ. The value is then cast through timestamptz(6), which treats it as session-local time and shifts it to UTC, so results are off-by-one under any non-UTC session time_zone. Move the TimeStampTz signature to last in each file. Literal-with-timezone arguments still route to TIMESTAMPTZ via the timeZoneCoersionScore branch, and typed TIMESTAMPTZ / DATETIMEV2 / DATEV2 arguments still bind their identical-match signatures; only the previously order-decided tie for non-literal string arguments changes, to DATETIMEV2 (wall-clock semantics). New DiffFunctionSignatureTest covers all 12 functions in both directions: varchar slots bind DATETIMEV2 (fails before this change), timestamptz slots still bind TIMESTAMPTZ. Fixes apache#66120
lets-order-some-fries
requested review from
924060929,
englefly,
morrySnow and
starocean999
as code owners
August 27, 2026 18:27
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What problem does this PR solve?
Issue Number: close #66120
Related PR: #64127
Problem Summary:
DATEDIFFand the other eleven*_diffscalar functions silently return off-by-one (or off-by-N) results when (1) an argument is a string-typed non-literal — a varchar column, subquery projection slot, or UNION-ALL output slot — and (2) the sessiontime_zoneis not UTC.All 12 functions list their TIMESTAMPTZ signature first in
SIGNATURES. The timezone-coercion penalty inSearchSignature.doMatchTypesonly fires for literals it can inspect (ExpressionUtils.getLiteralAfterUnwrapNullable, added in #64127), so for a varcharSlotReferenceevery candidate signature ties and the tie-break keeps the first-listed candidate — TIMESTAMPTZ. The varchar value is then cast throughtimestamptz(6), which treats it as session-local time and shifts it to UTC, so day-number arithmetic runs on the shifted value.Fix, as proposed by @Baymine in #66120: move the TimeStampTz signature to last in each of the 12
*Difffiles. Literal-with-timezone arguments still route to TIMESTAMPTZ via thetimeZoneCoersionScorebranch; typed TIMESTAMPTZ / DATETIMEV2 / DATEV2 arguments still bind their identical-match signatures. Only the previously order-decided tie for non-literal string arguments changes: TIMESTAMPTZ → DATETIMEV2 (wall-clock semantics, independent of session time zone).New unit test
DiffFunctionSignatureTestcovers all 12 functions in both directions: varchar slots must bind DATETIMEV2 (fails on master before this change) and TIMESTAMPTZ slots must still bind TIMESTAMPTZ. Happy to also add a SQL regression suite mirroring the issue's reproducer (varchar column / subquery slot / UNION-ALL slot under+08:00) if wanted — omitted here because the.outexpected-files need a cluster run to generate.Release note
Fixed DATEDIFF and the other *_diff functions returning wrong results on varchar columns and subquery slots when the session time_zone is not UTC.
Check List (For Author)
Test
Behavior changed:
*_diffcalls whose string-typed argument is NOT a literal, the argument now binds to DATETIMEV2 (wall-clock) instead of TIMESTAMPTZ (UTC-shifting). That re-bind is the fix; literal arguments and typed date/datetime/timestamptz arguments are unaffected.Does this need documentation?