github-actions[bot] commented on code in PR #69271:
URL: https://github.com/apache/doris/pull/69271#discussion_r4321407645
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java:
##########
@@ -606,20 +600,20 @@ public static Expression
castIfNotSameTypeStrict(Expression input, DataType targ
}
}
- /** Cast a DATETIME-family value to TIMESTAMP_NS and abort on conversion
overflow. */
+ /** Cast a date-like value to TIMESTAMP_NS and abort on conversion
overflow. */
public static Expression
castIfNotSameTypeForTimeStampNsCoercion(Expression input, DataType targetType) {
if (hasTimeStampNsType(targetType)
- && hasTimeStampNsCompatibleDateTimeType(input.getDataType())) {
+ && hasOtherDateLikeType(input.getDataType())) {
Review Comment:
[P2] Match corresponding nested fields before forcing strictness
With `enable_strict_cast=false`, consider `CASE WHEN flag THEN s ELSE n
END`, where `s` is `STRUCT<d:DATEV2,v:BIGINT>` containing
`('2024-01-01',25000101000000)` and `n` is `STRUCT<d:DATEV2,v:TIMESTAMP_NS>`
with an in-range timestamp. The reduced plan `Project(CASE ... THEN s ELSE n)
-> Scan` becomes `CASE ... THEN strict CAST(s AS STRUCT<DATEV2,TIMESTAMP_NS>)
ELSE n`. The DATEV2 field is unchanged; only BIGINT converts. This condition
finds DATEV2 in source field 1 and TIMESTAMP_NS in target field 2
independently, so it makes that unrelated numeric conversion strict and raises
an error instead of producing its normal NULL field. The previous helper used a
non-strict cast here. Please check corresponding source/target leaves for an
actual date-like-to-TIMESTAMP_NS conversion, including the equivalent strict
helper and `castInputs`, and cover this case 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] Propagate errors from the newly fallible ASOF probe expression
This promotion also inserts strict casts into ASOF `MATCH_CONDITION`, whose
probe path still assumes expression evaluation cannot fail. For example, with a
left DATETIMEV2 row `(k=1, dt='2500-01-01')` and a nonempty right TIMESTAMP_NS
row `(k=1, ts='2024-01-01')`, `l ASOF LEFT JOIN r MATCH_CONDITION(l.dt >= r.ts)
ON l.k=r.k` becomes `ASOF[k=k; strict CAST(l.dt AS TIMESTAMP_NS) >=
r.ts](Scan(l),Scan(r))`. `PhysicalPlanTranslator` sends the cast directly, and
`process_hash_table_probe_impl.h:284-285` replaces its returned overflow status
with `DORIS_CHECK(st.ok())`. In supported DEBUG/ASAN builds,
`Status::FatalError` calls `LOG(FATAL)` because those modes omit `NDEBUG`, so
ordinary out-of-range data terminates the BE instead of returning a conversion
error. Release catches a generic fatal query error and loses the overflow
detail. Please propagate the probe status through the normal error boundary and
add negative ASOF tests for both operand sides.
--
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]
