kgyrtkirk commented on a change in pull request #1044: [CALCITE-2838]
Simplification: Remove redundant IS TRUE/IS NOT FALSE …
URL: https://github.com/apache/calcite/pull/1044#discussion_r257592336
##########
File path:
core/src/test/java/org/apache/calcite/test/fuzzer/RexProgramFuzzyTest.java
##########
@@ -262,11 +262,12 @@ private void checkUnknownAs(RexNode node, RexUnknownAs
unknownAs) {
}
}
}
- if (opt.getType().isNullable() && !node.getType().isNullable()) {
+ if (unknownAs == RexUnknownAs.UNKNOWN
+ && opt.getType().isNullable()
Review comment:
@danny0405 thank you for taking a look
I would like to try to convince you that that will not happen:
* simplification in non-UNKNOWN mode should not have any effect on a
RelNode's rowType; there is matchNullability in ReduceExpressions which is
another line of defense against that; this test is running directly RexSimplify.
* a Project may never run with anything other than RexUnknownAs.UNKNOWN mode
- that would lead to interesting things quickly
* setting RexUnknownAs to anything which is not UNKNOWN means that it's
doing something closely related to boolean logic: it might be a Filter or a
Join condition both of these are setting unknownAs.FALSE mode because both of
them operates in an implicit "condtion IS TRUE" fashion
* I think one way to "remove" this unknownAs option would be to force the
special call sites; like join/filter to add an "is true" around the condition -
and then the actual filter/join implementation may remove the extra "IS TRUE"
clause; if it could operate in UnknownAs.FALSE mode
* the good side of this would be that: there would be no UnknownAs anymore
outside of RexSimplify and that Filter/Join would always seen a non-nullable
boolean type at all time.
* by this patch It may only change between boolean non-nullable to boolean
nullable ; because the input mode "UnknownAs.FALSE" means that null will be
handled as false.
* I've taken a look at all the callsites which might lead to a RexSimplify
running with `RexUnknownAs.FALSE` - and all of them have been coming from
ReduceExpressionsRule
* for Calc, Project: with RexUnknownAs.UNKNOWN
* for Join, Filter: with RexUnknownAs.FALSE
Right now I think that this will not have any adverse effects.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
