github-actions[bot] commented on code in PR #64639:
URL: https://github.com/apache/doris/pull/64639#discussion_r3437226560
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/util/ExpressionUtils.java:
##########
@@ -775,7 +775,7 @@ public static boolean canInferNotNullForMarkSlot(Expression
predicate, Expressio
return false;
}
}
- return true;
+ return meetNullOrFalse;
Review Comment:
With this new return value, the existing `CanInferNotNullForMarkSlotTest` no
longer matches the helper contract. For example, `new Or(BooleanLiteral.TRUE,
markSlot1)` folds to TRUE for both tested substitutions (`markSlot1 = NULL` and
`markSlot1 = FALSE`), so the loop only sets `meetTrue`; `meetNullOrFalse` stays
false and this method now returns false, while
`fe/fe-core/src/test/java/org/apache/doris/nereids/util/CanInferNotNullForMarkSlotTest.java:61`
still asserts true. Several surrounding comments also still say the all-TRUE
class is inferable. Please update the unit test and the
`ExpressionUtils`/`SubqueryToApply`/`TrySimplifyPredicateWithMarkJoinSlot`
comments to document the new valid class, or keep accepting all-TRUE cases if
that behavior is still intended. As-is the FE unit test coverage is
inconsistent with this change.
--
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]
