jcamachor commented on a change in pull request #2094:
URL: https://github.com/apache/calcite/pull/2094#discussion_r516184828



##########
File path: 
core/src/main/java/org/apache/calcite/plan/RelOptMaterializations.java
##########
@@ -50,6 +50,12 @@
  */
 public abstract class RelOptMaterializations {
 
+  @Deprecated // to be removed before 2.0
+  public static List<Pair<RelNode, List<RelOptMaterialization>>> 
useMaterializedViews(
+      final RelNode rel, List<RelOptMaterialization> materializations) {
+    return useMaterializedViews(rel, materializations, null);

Review comment:
       Should the third parameter be an empty list rather than null (I think 
otherwise we may hit an NPE)?

##########
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:
       Instead of having a mechanism to add _extra_ materialization rules, have 
we thought about adding all materialization rules through this mechanism? That 
way we can actually customize the full behavior rather than having a subset of 
internally defined rules that are hardcoded.




----------------------------------------------------------------
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]


Reply via email to