zabetak commented on a change in pull request #2094:
URL: https://github.com/apache/calcite/pull/2094#discussion_r521986581
##########
File path: core/src/main/java/org/apache/calcite/plan/AbstractRelOptPlanner.java
##########
@@ -197,6 +198,10 @@ public boolean isRuleExcluded(RelOptRule rule) {
return this;
}
+ @Override public void addExtraMaterializationRules(List<UnifyRule> rules) {
Review comment:
I also share @jcamachor view point. If we decide to allow customization
of this part its better to allow full-control.
Furthermore, to be consistent with the existing APIs of the planner, I would
opt for a per rule addition:
`addMaterializationRule(UnifyRule rule)`
Going one step further, is it really necessary to introduce a new API for
this? The point is that we want to add new rules to the planner so can't we use
the existing `addRule(RelOptRule)` interface? I guess we want to distinguish
those rules that are specific to materialization but this could be done in the
implementation of `addRule`.
What do you think?
----------------------------------------------------------------
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]
