asolimando commented on code in PR #4078:
URL: https://github.com/apache/calcite/pull/4078#discussion_r1873071546
##########
core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java:
##########
@@ -322,7 +325,9 @@ void register(MutableRel result, MutableRel query) {
ImmutableList.of(condition2, target2));
RexNode r =
canonizeNode(rexBuilder, simplify.simplifyUnknownAsFalse(x2));
- if (!r.isAlwaysFalse() && isEquivalent(condition2, r)) {
+ RexNode simplifiedCond2 =
+ canonizeNode(rexBuilder,
simplify.simplifyUnknownAsFalse(condition2));
+ if (!r.isAlwaysFalse() && isEquivalent(simplifiedCond2, r)) {
Review Comment:
I agree with @suibianwanwank, we shouldn't need another simplification on
`condition2` while early return seems good.
Here we are dealing with filtering predicates, so I guess that
`simplifyUnknownAsFalse` has the right semantics, rather than just `simplify`,
but we need tests covering these changes and showing that we are doing the
right thing on corner cases involving `unknown` and the early return condition.
@hannerwang, can you add them?
--
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]
