vlsi commented on a change in pull request #2253:
URL: https://github.com/apache/calcite/pull/2253#discussion_r540093603
##########
File path:
core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java
##########
@@ -366,14 +368,18 @@ public JoinReduceExpressionsRule(Class<? extends Join>
joinClass,
@Override public void onMatch(RelOptRuleCall call) {
final Join join = call.rel(0);
- final List<RexNode> expList = Lists.newArrayList(join.getCondition());
+ final RexBuilder rexBuilder = join.getCluster().getRexBuilder();
+ final RexSimplify rexSimplify =
+ new RexSimplify(rexBuilder, RelOptPredicateList.EMPTY, EXECUTOR);
+ final RexNode condition =
rexSimplify.eliminateCommonExprInCondition(join.getCondition());
+
+ final List<RexNode> expList = Lists.newArrayList(condition);
Review comment:
I agree with @julianhyde that "CNF rewrite only in the particular rule".
However, I believe it should be implemented as a flag in RexSimplify that
would optionally enable "CNF rewrite".
The rules that need CNF rewrite would set the flag.
The current implementation leaves the door open for the fight between
different simplifications: the regular simplifier might want to simplify in its
own way (e.g. ReduceExpressionsRule), and JoinConditionSimplify rule might want
to rewrite into CNF.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
