chunweilei commented on a change in pull request #1686: [CALCITE-3630] Improve
ReduceExpressionsRule
URL: https://github.com/apache/calcite/pull/1686#discussion_r361576836
##########
File path:
core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java
##########
@@ -270,6 +270,8 @@ private void reduceNotNullableFilter(
} else {
call.transformTo(createEmptyRelOrEquivalent(call, filter));
}
+ // New plan is absolutely better than old plan.
+ call.getPlanner().setImportance(filter, 0.0);
Review comment:
> Assume that we enter the else branch and we create an empty LogicalValues.
Then if the rule set does not contain a PhysicalValues we may end-up with a
CannotPlanException.
This is not valid. Because without this change, `ReduceExpressionsRule` can
also create an empty `LogicalValues`[1]. I agree that we should pay more
attention when we use `setImportance`. But as I mentioned before, what this
change does is to be consistent with what the if branch does.
[1]
https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java#L199
----------------------------------------------------------------
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]
With regards,
Apache Git Services
