hsyuan commented on a change in pull request #1869: [CALCITE-3868] Remove 
redundant ruleSet and ruleNames in VolcanoPlanner
URL: https://github.com/apache/calcite/pull/1869#discussion_r399585261
 
 

 ##########
 File path: 
core/src/main/java/org/apache/calcite/plan/AbstractRelOptPlanner.java
 ##########
 @@ -44,18 +44,13 @@
  * Abstract base for implementations of the {@link RelOptPlanner} interface.
  */
 public abstract class AbstractRelOptPlanner implements RelOptPlanner {
-  //~ Static fields/initializers ---------------------------------------------
-
-  /** Regular expression for integer. */
-  private static final Pattern INTEGER_PATTERN = Pattern.compile("[0-9]+");
-
   //~ Instance fields --------------------------------------------------------
 
   /**
    * Maps rule description to rule, just to ensure that rules' descriptions
    * are unique.
    */
-  private final Map<String, RelOptRule> mapDescToRule = new HashMap<>();
+  protected final Map<String, RelOptRule> mapDescToRule = new HashMap<>();
 
 Review comment:
   I agree the rules is not very big. But in `onNewClass`, every time we add a 
new logical/physical operator we have to copy it. It will copy N times depends 
on how many operators you have. Though not a significant overhead, but I think 
we still need to avoid multiple times copy.

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

Reply via email to