github-actions[bot] commented on code in PR #69271:
URL: https://github.com/apache/doris/pull/69271#discussion_r4311861319
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java:
##########
@@ -1154,17 +1148,14 @@ private static Optional<DataType>
getCommonDataTypeWithDateTimeV2Type(
return Optional.empty();
}
- /** Choose TIMESTAMP_NS for compatible DATETIME-family coercion. */
+ /** Choose TIMESTAMP_NS for compatible date-like coercion. */
private static Optional<DataType>
getCommonDataTypeWithTimeStampNsType(DataType otherType) {
if (otherType instanceof TimeStampNsType) {
return Optional.of(TimeStampNsType.INSTANCE);
}
- if (isTimeStampNsCompatibleDateTimeType(otherType)) {
+ if (isOtherDateLikeType(otherType)) {
Review Comment:
[P2] Preserve strict casts for mixed temporal array comparisons
The new DATE/DATEV2 common type also enables comparisons between
`ARRAY<DATEV2>` and `ARRAY<TIMESTAMP_NS>`, but
`processComparisonPredicateInternal` only recognizes scalar temporal pairs. For
`SELECT dates = stamps FROM t` with `dates = ['2500-01-01']`, the reduced plan
becomes `Project(CAST(dates AS ARRAY<TIMESTAMP_NS>) = stamps) -> Scan(t)` with
`isStrict=false`. With `enable_strict_cast=false`, the BE array wrapper
consequently converts that date to a NULL element and returns a comparison
result instead of raising the required overflow. IN, CASE and set operations
already use the recursive strict helper. Please use that helper for comparison
operands too, and cover nested DATE/DATEV2 comparisons in both coercion modes.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java:
##########
@@ -1533,28 +1523,13 @@ private static Expression
processComparisonPredicateInternal(ComparisonPredicate
left = comparisonPredicate.left();
right = comparisonPredicate.right();
- if (isJoinCondition && comparisonPredicate instanceof EqualPredicate
- && isTimeStampNsAndJoinCompatibleTemporalPair(
- left.getDataType(), right.getDataType())) {
- left = strictCastIfNotSameType(left, TimeStampNsType.INSTANCE);
- right = strictCastIfNotSameType(right, TimeStampNsType.INSTANCE);
- return comparisonPredicate.withChildren(left, right);
- }
-
boolean timeStampNsDateLikePair = isTimeStampNsAndDateLikePair(
left.getDataType(), right.getDataType());
Optional<DataType> commonType;
if (timeStampNsDateLikePair) {
- commonType = findExactCommonTypeForTimeStampNsAndDateLike(
- ImmutableList.of(left, right));
- // The BE comparison kernel compares TIMESTAMP_NS and DATETIMEV2
exactly. Date-like
- // peers with no exact common type can first be widened losslessly
to DATETIMEV2.
- if (!commonType.isPresent()) {
- Optional<Expression> normalized =
normalizeTimeStampNsDateLikeComparison(comparisonPredicate);
- if (normalized.isPresent()) {
- return normalized.get();
- }
- }
+ // Storage predicates require the constant and column to share a
representation.
+ // Always promote the temporal peer, preserving nanoseconds and
failing on overflow.
+ commonType = Optional.of(TimeStampNsType.INSTANCE);
Review Comment:
[P2] Migrate existing regression cases to the new overflow contract
The stricter comparison behavior leaves existing timestamp suites failing.
`test_timestamp_ns_partition_bucket.groovy:273-279` still runs a `qt_`
projection of `dt = TIMESTAMP_NS_literal` for the row whose `dt` is `9999-12-31
23:59:59.999999`, and its snapshot expects `4 false`. This path now inserts a
strict cast and raises `TIMESTAMP_NS overflow`; the projected expression cannot
be removed by partition pruning. Likewise,
`test_timestamp_ns_expressions.groovy:156-166` still expects successful TRUE
results for mixed comparisons against DATETIMEV2 literals in years 9999 and
1600. Please migrate these cases to `test { sql ...; exception ... }` and
regenerate their result files alongside the three updated suites.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/SearchSignature.java:
##########
@@ -267,33 +266,22 @@ private Pair<Boolean, Pair<Integer, Integer>>
doMatchTypes(FunctionSignature sig
DataType sigArgType = sig.getArgType(i);
Expression argument = arguments.get(i);
DataType realType = argument.getDataType();
- if (hasTimeStampNsArgument &&
hasTimeStampNsCompatibleDateTimeArgument && hasDateLikeSignature
+ if (hasTimeStampNsArgument && hasOtherDateLikeArgument &&
hasDateLikeSignature
&& realType.isDateLikeType() &&
!sigArgType.isDateLikeType()) {
// Do not bypass temporal exactness checks through a generic
string overload.
return Pair.of(false, Pair.of(stringLiteralCoersionCount,
timeZoneCoersionScore));
}
- if (hasTimeStampNsArgument &&
hasTimeStampNsCompatibleDateTimeArgument && hasDateLikeSignature
- && (realType.isTimeStampNsType() ||
realType.isDateTimeType()
- || realType.isDateTimeV2Type() ||
realType.isTimeStampTzType())
- && sigArgType.isDateLikeType() &&
!sigArgType.isTimeStampNsType()
- && !(computeSignature instanceof
SupportsMixedTimeStampNsDateTime)) {
- // Functions that need a common temporal type use the
TIMESTAMP_NS overload.
- // Functions without a common temporal result opt in to mixed
physical signatures.
+ if (hasTimeStampNsArgument && hasOtherDateLikeArgument &&
hasDateLikeSignature
Review Comment:
[P2] Preserve exact declared UDF signatures with mixed temporal arguments
This gate also runs for `AliasUdf` and `JavaUdf`, whose arguments need not
share a temporal type. An alias declared as `CREATE ALIAS FUNCTION
render_event(TIMESTAMP_NS, DATEV2) WITH PARAMETER(ts,d) AS concat(ts,'|',d)`
can no longer be called even with exactly those argument types:
`AliasUdfBuilder` invokes `processBoundFunction` before expansion, and this
branch rejects its declared DATEV2 parameter in every signature-search round.
The previous gate did not trigger for DATE/DATEV2, so this is a regression even
for values inside the nanosecond range. Please limit common-type forcing to the
relevant builtin argument groups, while allowing exact declared UDF signatures.
--
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]
