lets-order-some-fries commented on code in PR #67238:
URL: https://github.com/apache/doris/pull/67238#discussion_r4403254200
##########
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:
Pushed 7056a0d2a7. The revision keeps `SearchSignature.java` and the 12
`SIGNATURES` lists as on master and scopes the fix to the `*_diff` functions
through a new mixin, `SearchSignatureForDateDiff` (the
`SearchSignatureForRound` pattern): only when a call has a string-typed
argument and no string literal to inspect is the TIMESTAMPTZ overload tried
after the others. Any call with an inspectable literal goes through the
unchanged search, so both mixed cases keep TIMESTAMPTZ: under
`America/Los_Angeles`, `hours_diff('2021-03-15 00:00:00-0700', '2021-03-14
00:00:00-08:00')` stays 23 (reversed -23) and `hours_diff('2021-03-14
03:30:00-07:00', '2021-03-14 01:30:00')` stays 1.
`testCompactAndColonOffsetLiteralsKeepTimeStampTz` and
`testMixedZonedAndZonelessLiteralsKeepTimeStampTz` are back, with three
boundary tests beside them, and `test_timestamptz_diff_string_binding.groovy`
runs those queries plus the issue's reproducer against a VARCHAR column, a
subquery slot and a UNION-ALL slot. Locally, 94 fe-core tests pass across the
six datetime-binding classes, checkstyle is clean, and two mutation runs show
each half of the new condition is pinned by a test; the regression suite needs
the cluster. Could you approve the workflows and run buildall and /review when
you have a moment?
--
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]
