924060929 commented on code in PR #67238:
URL: https://github.com/apache/doris/pull/67238#discussion_r4384457645
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/HoursDiff.java:
##########
@@ -42,11 +42,11 @@ public class HoursDiff extends ScalarFunction implements
BinaryExpression, Expli
PropagateNullable, DateDiffMonotonic {
public static final List<FunctionSignature> SIGNATURES = ImmutableList.of(
- FunctionSignature.ret(BigIntType.INSTANCE)
- .args(TimeStampTzType.WILDCARD, TimeStampTzType.WILDCARD),
FunctionSignature.ret(BigIntType.INSTANCE)
.args(DateTimeV2Type.WILDCARD, DateTimeV2Type.WILDCARD),
-
FunctionSignature.ret(BigIntType.INSTANCE).args(DateV2Type.INSTANCE,
DateV2Type.INSTANCE));
+
FunctionSignature.ret(BigIntType.INSTANCE).args(DateV2Type.INSTANCE,
DateV2Type.INSTANCE),
+ FunctionSignature.ret(BigIntType.INSTANCE)
+ .args(TimeStampTzType.WILDCARD, TimeStampTzType.WILDCARD));
Review Comment:
[P1] The latest revision reintroduces the mixed-offset regression described
above.
For the current head `40cb6e82`, consider two literals that both carry
explicit offsets:
```sql
SET time_zone = 'America/Los_Angeles';
SELECT hours_diff(
'2021-03-15 00:00:00-0700',
'2021-03-14 00:00:00-08:00'
);
```
Their UTC values are `2021-03-15 07:00:00` and `2021-03-14 08:00:00`, so the
elapsed difference is 23 hours. Tracing binding and cast behavior gives:
- Master at the PR merge base `f1fd1b432f`: the timezone scores cancel to
zero, but TIMESTAMPTZ is listed first, so it wins and the UTC difference is
**23 hours**.
- This head: the scores still cancel, but DATETIMEV2 is now first. Both
inputs become session-local midnights and the civil difference is **24 hours**.
- Replacing only `-0700` with the equivalent spelling `-07:00` makes both
zones recognizable, selects TIMESTAMPTZ and gives **23 hours** again.
The failure to recognize compact offsets is pre-existing; changing this
previously working mixed-offset call from 23 to 24 hours is introduced by this
PR. I have now verified the SQL through separately built and started Doris FE
servers (2026-10-27). `SHOW FRONTENDS` confirmed the runtime commits: fetched
master `97ae75cb12f` and PR `40cb6e8218e`.
| Executed query | master `97ae75cb12f` | PR `40cb6e82` |
|---|---:|---:|
| Original compact + colon offsets | 23 | 24 |
| Equivalent colon + colon offsets | 23 | 23 |
| Explicit TIMESTAMPTZ casts | 23 | 23 |
| Reversed original arguments | -23 | -24 |
| Mixed zoned + zone-less DST example from the original review | 1 | 2 |
Both servers used `time_zone='America/Los_Angeles'` and
`enable_nereids_distribute_planner=false`, with constant folding enabled. These
were actual MySQL queries executed and returned by FE, with no BE registered; I
am not claiming BE execution with folding disabled. With
`debug_skip_fold_constant=true`, EXPLAIN independently showed TIMESTAMPTZ(6)
casts on master versus DATETIMEV2(6) on the PR. The final comparison also
checked runtime commit metadata after refreshing the build-version generation
cache.
Commit `40cb6e82` also deletes
`testCompactAndColonOffsetLiteralsKeepTimeStampTz` entirely, and replaces
`testMixedZonedAndZonelessLiteralsKeepTimeStampTz` with
`testMixedZonedAndZonelessLiteralsBindDateTimeV2`, reversing its expected
argument types. Thus the tests no longer guard the behavior protected at
reviewed head `990882a3`.
Restoring the shared scorer to avoid changing ceil/floor is reasonable, but
that does not justify changing existing diff results. Please preserve the
existing literal/instant behavior while fixing the uninspectable VARCHAR-slot
fallback, without changing ceil/floor globally, and restore the mixed-offset
coverage with an execution-level DST regression. Declaring the behavior change
in the PR body does not remove this compatibility issue.
--
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]
