[CALCITE-4111] Remove VolcanoPlannerPhase in Planner (Jiatao Tao)

Close apache#2076

core/src/main/java/org/apache/calcite/plan/volcano/IterativeRuleDriver.java

@@ -22,11 +22,10 @@
 import org.slf4j.Logger;
 
 /***
- * <p>The algorithm executes repeatedly in a series of phases. In each phase
- * the exact rules that may be fired varies. The mapping of phases to rule
- * sets is maintained in the {@link #ruleQueue}.
+ * <p>The algorithm executes repeatedly. The exact rules
+ * that may be fired varies.
  *
- * <p>In each phase, the planner then iterates over the rule matches presented
+ * <p>The planner iterates over the rule matches presented
  * by the rule queue until the rule queue becomes empty.
  */
 class IterativeRuleDriver implements RuleDriver {
@@ -46,34 +45,28 @@ class IterativeRuleDriver implements RuleDriver {
   }
 
   @Override public void drive() {
-    PLANNING:
-    for (VolcanoPlannerPhase phase : VolcanoPlannerPhase.values()) {
-      while (true) {
-        LOGGER.debug("PLANNER = {}; PHASE = {}; COST = {}",
-            this, phase.toString(), planner.root.bestCost);
+    while (true) {
+      LOGGER.debug("PLANNER = {}; COST = {}", this, planner.root.bestCost);
 
-        VolcanoRuleMatch match = ruleQueue.popMatch(phase);
-        if (match == null) {
-          break;
-        }
-
-        assert match.getRule().matches(match);
-        try {
-          match.onMatch();
-        } catch (VolcanoTimeoutException e) {
-          LOGGER.warn("Volcano planning times out, cancels the subsequent optimization.");
-          planner.canonize();
-          ruleQueue.phaseCompleted(phase);
-          break PLANNING;
-        }
+      VolcanoRuleMatch match = ruleQueue.popMatch();
+      if (match == null) {
+        break;
+      }
 
-        // The root may have been merged with another
-        // subset. Find the new root subset.
+      assert match.getRule().matches(match);
+      try {
+        match.onMatch();
+      } catch (VolcanoTimeoutException e) {
+        LOGGER.warn("Volcano planning times out, cancels the subsequent optimization.");
         planner.canonize();
+        break;
       }
 
-      ruleQueue.phaseCompleted(phase);
+      // The root may have been merged with another
+      // subset. Find the new root subset.
+      planner.canonize();
     }
+
   }
 
   @Override public void onProduce(RelNode rel, RelSubset subset) {

core/src/main/java/org/apache/calcite/plan/volcano/IterativeRuleQueue.java

@@ -20,17 +20,14 @@
 import org.apache.calcite.util.trace.CalciteTrace;
 
 import com.google.common.collect.HashMultimap;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Multimap;
 
 import org.slf4j.Logger;
 
 import java.io.PrintWriter;
 import java.io.StringWriter;
-import java.util.EnumMap;
 import java.util.HashSet;
 import java.util.LinkedList;
-import java.util.Map;
 import java.util.Queue;
 import java.util.Set;
 
@@ -43,56 +40,20 @@ class IterativeRuleQueue extends RuleQueue {
 
   private static final Logger LOGGER = CalciteTrace.getPlannerTracer();
 
-  private static final Set<String> ALL_RULES = ImmutableSet.of("<ALL RULES>");
-
   //~ Instance fields --------------------------------------------------------
 
   /**
-   * Map of {@link VolcanoPlannerPhase} to a list of rule-matches. Initially,
-   * there is an empty {@link PhaseMatchList} for each planner phase. As the
-   * planner invokes {@link #addMatch(VolcanoRuleMatch)} the rule-match is
-   * added to the appropriate PhaseMatchList(s). As the planner completes
-   * phases, the matching entry is removed from this list to avoid unused
-   * work.
-   */
-  final Map<VolcanoPlannerPhase, PhaseMatchList> matchListMap =
-      new EnumMap<>(VolcanoPlannerPhase.class);
-
-  /**
-   * Maps a {@link VolcanoPlannerPhase} to a set of rule descriptions. Named rules
-   * may be invoked in their corresponding phase.
-   *
-   * <p>See {@link VolcanoPlannerPhaseRuleMappingInitializer} for more
-   * information regarding the contents of this Map and how it is initialized.
+   * The list of rule-matches. Initially, there is an empty {@link MatchList}.
+   * As the planner invokes {@link #addMatch(VolcanoRuleMatch)} the rule-match
+   * is added to the appropriate MatchList(s). As the planner completes the
+   * match, the matching entry is removed from this list to avoid unused work.
    */
-  private final Map<VolcanoPlannerPhase, Set<String>> phaseRuleMapping;
+  final MatchList matchList = new MatchList();
 
   //~ Constructors -----------------------------------------------------------
 
   IterativeRuleQueue(VolcanoPlanner planner) {
     super(planner);
-
-    phaseRuleMapping = new EnumMap<>(VolcanoPlannerPhase.class);
-
-    // init empty sets for all phases
-    for (VolcanoPlannerPhase phase : VolcanoPlannerPhase.values()) {
-      phaseRuleMapping.put(phase, new HashSet<>());
-    }
-
-    // configure phases
-    planner.getPhaseRuleMappingInitializer().initialize(phaseRuleMapping);
-
-    for (VolcanoPlannerPhase phase : VolcanoPlannerPhase.values()) {
-      // empty phases get converted to "all rules"
-      if (phaseRuleMapping.get(phase).isEmpty()) {
-        phaseRuleMapping.put(phase, ALL_RULES);
-      }
-
-      // create a match list data structure for each phase
-      PhaseMatchList matchList = new PhaseMatchList(phase);
-
-      matchListMap.put(phase, matchList);
-    }
   }
 
   //~ Methods ----------------------------------------------------------------
@@ -101,51 +62,30 @@ class IterativeRuleQueue extends RuleQueue {
    */
   @Override public boolean clear() {
     boolean empty = true;
-    for (PhaseMatchList matchList : matchListMap.values()) {
-      if (!matchList.queue.isEmpty() || !matchList.preQueue.isEmpty()) {
-        empty = false;
-      }
-      matchList.clear();
+    if (!matchList.queue.isEmpty() || !matchList.preQueue.isEmpty()) {
+      empty = false;
     }
+    matchList.clear();
     return !empty;
   }
 
   /**
-   * Removes the {@link PhaseMatchList rule-match list} for the given planner
-   * phase.
-   */
-  public void phaseCompleted(VolcanoPlannerPhase phase) {
-    matchListMap.get(phase).clear();
-  }
-
-  /**
-   * Adds a rule match. The rule-matches are automatically added to all
-   * existing {@link PhaseMatchList per-phase rule-match lists} which allow
-   * the rule referenced by the match.
+   * Add a rule match.
    */
   public void addMatch(VolcanoRuleMatch match) {
     final String matchName = match.toString();
-    for (PhaseMatchList matchList : matchListMap.values()) {
-      Set<String> phaseRuleSet = phaseRuleMapping.get(matchList.phase);
-      if (phaseRuleSet != ALL_RULES) {
-        String ruleDescription = match.getRule().toString();
-        if (!phaseRuleSet.contains(ruleDescription)) {
-          continue;
-        }
-      }
 
-      if (!matchList.names.add(matchName)) {
-        // Identical match has already been added.
-        continue;
-      }
+    if (!matchList.names.add(matchName)) {
+      // Identical match has already been added.
+      return;
+    }
 
-      LOGGER.trace("{} Rule-match queued: {}", matchList.phase.toString(), matchName);
+    LOGGER.trace("Rule-match queued: {}", matchName);
 
-      matchList.offer(match);
+    matchList.offer(match);
 
-      matchList.matchMap.put(
-          planner.getSubset(match.rels[0]), match);
-    }
+    matchList.matchMap.put(
+        planner.getSubset(match.rels[0]), match);
   }
 
   /**
@@ -155,30 +95,21 @@ public void addMatch(VolcanoRuleMatch match) {
    *
    * <p>Note that the VolcanoPlanner may still decide to reject rule matches
    * which have become invalid, say if one of their operands belongs to an
-   * obsolete set or has importance=0.
+   * obsolete set or has been pruned.
    *
-   * @throws java.lang.AssertionError if this method is called with a phase
-   *                              previously marked as completed via
-   *                              {@link #phaseCompleted(VolcanoPlannerPhase)}.
    */
-  public VolcanoRuleMatch popMatch(VolcanoPlannerPhase phase) {
+  public VolcanoRuleMatch popMatch() {
     dumpPlannerState();
 
-    PhaseMatchList phaseMatchList = matchListMap.get(phase);
-    if (phaseMatchList == null) {
-      throw new AssertionError("Used match list for phase " + phase
-          + " after phase complete");
-    }
-
     VolcanoRuleMatch match;
     for (;;) {
-      if (phaseMatchList.size() == 0) {
+      if (matchList.size() == 0) {
         return null;
       }
 
-      dumpRuleQueue(phaseMatchList);
+      dumpRuleQueue(matchList);
 
-      match = phaseMatchList.poll();
+      match = matchList.poll();
 
       if (skipMatch(match)) {
         LOGGER.debug("Skip match: {}", match);
@@ -191,7 +122,7 @@ public VolcanoRuleMatch popMatch(VolcanoPlannerPhase phase) {
     // may not be removed from the matchMap because the subset may have
     // changed, it is OK to leave it since the matchMap will be cleared
     // at the end.
-    phaseMatchList.matchMap.remove(
+    matchList.matchMap.remove(
         planner.getSubset(match.rels[0]), match);
 
     LOGGER.debug("Pop match: {}", match);
@@ -201,15 +132,15 @@ public VolcanoRuleMatch popMatch(VolcanoPlannerPhase phase) {
   /**
    * Dumps rules queue to the logger when debug level is set to {@code TRACE}.
    */
-  private void dumpRuleQueue(PhaseMatchList phaseMatchList) {
+  private void dumpRuleQueue(MatchList matchList) {
     if (LOGGER.isTraceEnabled()) {
       StringBuilder b = new StringBuilder();
       b.append("Rule queue:");
-      for (VolcanoRuleMatch rule : phaseMatchList.preQueue) {
+      for (VolcanoRuleMatch rule : matchList.preQueue) {
         b.append("\n");
         b.append(rule);
       }
-      for (VolcanoRuleMatch rule : phaseMatchList.queue) {
+      for (VolcanoRuleMatch rule : matchList.queue) {
         b.append("\n");
         b.append(rule);
       }
@@ -234,15 +165,9 @@ private void dumpPlannerState() {
   //~ Inner Classes ----------------------------------------------------------
 
   /**
-   * PhaseMatchList represents a set of {@link VolcanoRuleMatch rule-matches}
-   * for a particular
-   * {@link VolcanoPlannerPhase phase of the planner's execution}.
+   * MatchList represents a set of {@link VolcanoRuleMatch rule-matches}.
    */
-  private static class PhaseMatchList {
-    /**
-     * The VolcanoPlannerPhase that this PhaseMatchList is used in.
-     */
-    final VolcanoPlannerPhase phase;
+  private static class MatchList {
 
     /**
      * Rule match queue for SubstitutionRule
@@ -268,10 +193,6 @@ private static class PhaseMatchList {
     final Multimap<RelSubset, VolcanoRuleMatch> matchMap =
         HashMultimap.create();
 
-    PhaseMatchList(VolcanoPlannerPhase phase) {
-      this.phase = phase;
-    }
-
     int size() {
       return preQueue.size() + queue.size();
     }

core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java

@@ -237,16 +237,6 @@ private void initRuleQueue() {
 
   //~ Methods ----------------------------------------------------------------
 
-  protected VolcanoPlannerPhaseRuleMappingInitializer
-      getPhaseRuleMappingInitializer() {
-    return phaseRuleMap -> {
-      // Disable all phases except OPTIMIZE by adding one useless rule name.
-      phaseRuleMap.get(VolcanoPlannerPhase.PRE_PROCESS_MDR).add("xxx");
-      phaseRuleMap.get(VolcanoPlannerPhase.PRE_PROCESS).add("xxx");
-      phaseRuleMap.get(VolcanoPlannerPhase.CLEANUP).add("xxx");
-    };
-  }
-
   /**
    * Enable or disable top-down optimization.
    *