[CALCITE-557] Remove AbstractConverter

Also, make costing of joins more deterministic when both inputs have the same number of rows; order on the number of fields before ordering on RelNode.id.

core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableJoin.java

@@ -25,6 +25,7 @@
 import org.apache.calcite.plan.RelTraitSet;
 import org.apache.calcite.rel.InvalidRelException;
 import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.RelNodes;
 import org.apache.calcite.rel.core.JoinInfo;
 import org.apache.calcite.rel.core.JoinRelType;
 import org.apache.calcite.rel.metadata.RelMetadataQuery;
@@ -91,7 +92,7 @@ public class EnumerableJoin extends EquiJoin implements EnumerableRel {
       rowCount = addEpsilon(rowCount);
       break;
     default:
-      if (left.getId() > right.getId()) {
+      if (RelNodes.COMPARATOR.compare(left, right) > 0) {
         rowCount = addEpsilon(rowCount);
       }
     }

core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcConvention.java

@@ -43,6 +43,10 @@
  * instantiated, at the start of planning, for each JDBC database in play.</p>
  */
 public class JdbcConvention extends Convention.Impl {
+  /** Cost of a JDBC node versus implementing an equivalent node in a "typical"
+   * calling convention. */
+  public static final double COST_MULTIPLIER = 0.8d;
+
   public final SqlDialect dialect;
   public final Expression expression;
 

core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java

@@ -471,6 +471,11 @@ public JdbcProject(
           flags);
     }
 
+    @Override public RelOptCost computeSelfCost(RelOptPlanner planner) {
+      return super.computeSelfCost(planner)
+          .multiplyBy(JdbcConvention.COST_MULTIPLIER);
+    }
+
     public JdbcImplementor.Result implement(JdbcImplementor implementor) {
       JdbcImplementor.Result x = implementor.visitChild(0, getInput());
       if (isStar(exps, getInput().getRowType())) {

core/src/main/java/org/apache/calcite/interpreter/ScanNode.java

@@ -79,8 +79,11 @@ private Enumerable<Row> iterable() {
         table.unwrap(ProjectableFilterableTable.class);
     if (pfTable != null) {
       final List<RexNode> filters1 = Lists.newArrayList(filters);
+      final int[] projects1 =
+          isIdentity(projects, rel.getRowType().getFieldCount())
+              ? null : projects;
       final Enumerable<Object[]> enumerator =
-          pfTable.scan(root, filters1, projects);
+          pfTable.scan(root, filters1, projects1);
       assert filters1.isEmpty()
           : "table could not handle a filter it earlier said it could";
       return Enumerables.toRow(enumerator);
@@ -153,6 +156,18 @@ public Row apply(Object o) {
     }
     throw new AssertionError("cannot convert table " + table + " to iterable");
   }
+
+  private static boolean isIdentity(int[] is, int count) {
+    if (is.length != count) {
+      return false;
+    }
+    for (int i = 0; i < is.length; i++) {
+      if (is[i] != i) {
+        return false;
+      }
+    }
+    return true;
+  }
 }
 
 // End ScanNode.java

core/src/main/java/org/apache/calcite/materialize/Lattice.java

@@ -583,6 +583,10 @@ public int compareTo(Column column) {
           && this.ordinal == ((Column) obj).ordinal;
     }
 
+    @Override public String toString() {
+      return identifiers().toString();
+    }
+
     public List<String> identifiers() {
       return ImmutableList.of(table, column);
     }

core/src/main/java/org/apache/calcite/materialize/MaterializationService.java

@@ -262,12 +262,7 @@ && allSatisfiable(measureList, tileKey2)) {
         }
       }
       if (!queue.isEmpty()) {
-        final Pair<CalciteSchema.TableEntry, TileKey> best = queue.peek();
-        for (Pair<CalciteSchema.TableEntry, TileKey> pair : queue) {
-          System.out.println("table=" + pair.left.path() + " "
-              + pair.left.getTable().getStatistic().getRowCount());
-        }
-        return best;
+        return queue.peek();
       }
     }
 

core/src/main/java/org/apache/calcite/materialize/TileKey.java

@@ -51,6 +51,10 @@ public TileKey(Lattice lattice, ImmutableBitSet dimensions,
         && dimensions.equals(((TileKey) obj).dimensions)
         && measures.equals(((TileKey) obj).measures);
   }
+
+  @Override public String toString() {
+    return "dimensions: " + dimensions + ", measures: " + measures;
+  }
 }
 
 // End TileKey.java

core/src/main/java/org/apache/calcite/materialize/TileSuggester.java

@@ -75,7 +75,6 @@ public Iterable<? extends Lattice.Tile> tiles() {
     final Result result = algorithm.run(schema, map.build(), progress);
     final ImmutableList.Builder<Lattice.Tile> tiles = ImmutableList.builder();
     for (Aggregate aggregate : result.getAggregates()) {
-      System.out.println(aggregate);
       tiles.add(toTile(aggregate));
     }
     return tiles.build();

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

@@ -16,8 +16,6 @@
  */
 package org.apache.calcite.plan.volcano;
 
-import org.apache.calcite.plan.Convention;
-import org.apache.calcite.plan.ConventionTraitDef;
 import org.apache.calcite.plan.RelOptCluster;
 import org.apache.calcite.plan.RelOptListener;
 import org.apache.calcite.plan.RelOptUtil;
@@ -159,32 +157,8 @@ RelSubset getOrCreateSubset(
       final VolcanoPlanner planner =
           (VolcanoPlanner) cluster.getPlanner();
 
-      // Add converters to convert the new subset to each existing subset.
-      for (RelSubset subset1 : subsets) {
-        if (subset1.getConvention() == Convention.NONE) {
-          continue;
-        }
-        final AbstractConverter converter =
-            new AbstractConverter(
-                cluster, subset, ConventionTraitDef.INSTANCE,
-                subset1.getTraitSet());
-        planner.register(converter, subset1);
-      }
-
       subsets.add(subset);
 
-      // Add converters to convert each existing subset to this subset.
-      for (RelSubset subset1 : subsets) {
-        if (subset1 == subset) {
-          continue;
-        }
-        final AbstractConverter converter =
-            new AbstractConverter(
-                cluster, subset1, ConventionTraitDef.INSTANCE,
-                traits);
-        planner.register(converter, subset);
-      }
-
       if (planner.listener != null) {
         postEquivalenceEvent(planner, subset);
       }

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

@@ -34,6 +34,8 @@
 import org.apache.calcite.util.Util;
 import org.apache.calcite.util.trace.CalciteTrace;
 
+import com.google.common.collect.Iterables;
+
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.util.ArrayList;
@@ -156,7 +158,10 @@ public void explain(RelWriter pw) {
     String s = getDescription();
     pw.item("subset", s);
     final AbstractRelNode input =
-        (AbstractRelNode) getRels().iterator().next();
+        (AbstractRelNode) Iterables.getFirst(getRels(), null);
+    if (input == null) {
+      return;
+    }
     input.explainTerms(pw);
     pw.done(input);
   }

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

@@ -19,6 +19,7 @@
 import org.apache.calcite.plan.RelOptCost;
 import org.apache.calcite.plan.RelOptRuleOperand;
 import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.RelNodes;
 import org.apache.calcite.util.ChunkList;
 import org.apache.calcite.util.Stacks;
 import org.apache.calcite.util.Util;
@@ -91,7 +92,7 @@ class RuleQueue {
   /**
    * Sorts rule-matches into decreasing order of importance.
    */
-  private final Comparator<VolcanoRuleMatch> ruleMatchImportanceComparator =
+  private static final Comparator<VolcanoRuleMatch> MATCH_COMPARATOR =
       new RuleMatchImportanceComparator();
 
   private final VolcanoPlanner planner;
@@ -460,7 +461,7 @@ VolcanoRuleMatch popMatch(VolcanoPlannerPhase phase) {
         return null;
       }
       if (LOGGER.isLoggable(Level.FINEST)) {
-        Collections.sort(matchList, ruleMatchImportanceComparator);
+        Collections.sort(matchList, MATCH_COMPARATOR);
         match = matchList.remove(0);
 
         StringBuilder b = new StringBuilder();
@@ -483,7 +484,7 @@ VolcanoRuleMatch popMatch(VolcanoPlannerPhase phase) {
         for (VolcanoRuleMatch match2 : matchList) {
           ++i;
           if (match == null
-              || ruleMatchImportanceComparator.compare(match2, match) < 0) {
+              || MATCH_COMPARATOR.compare(match2, match) < 0) {
             bestPos = i;
             match = match2;
           }
@@ -613,20 +614,6 @@ private double toDouble(RelOptCost cost) {
     }
   }
 
-  static int compareRels(RelNode[] rels0, RelNode[] rels1) {
-    int c = rels0.length - rels1.length;
-    if (c != 0) {
-      return c;
-    }
-    for (int i = 0; i < rels0.length; i++) {
-      c = rels0[i].getId() - rels1[i].getId();
-      if (c != 0) {
-        return c;
-      }
-    }
-    return 0;
-  }
-
   private static double computeOneMinusEpsilon() {
     for (double d = 0d;;) {
       double d0 = d;
@@ -662,20 +649,22 @@ public int compare(
    * comparing the {@link RelNode#getId id}s of the relational expressions
    * matched.
    */
-  private class RuleMatchImportanceComparator
+  private static class RuleMatchImportanceComparator
       implements Comparator<VolcanoRuleMatch> {
-    public int compare(
-        VolcanoRuleMatch match1,
+    public int compare(VolcanoRuleMatch match1,
         VolcanoRuleMatch match2) {
+      int c = match1.rule.getClass().getName()
+          .compareTo(match2.rule.getClass().getName());
+      if (c != 0) {
+        return -c;
+      }
       double imp1 = match1.getImportance();
       double imp2 = match2.getImportance();
-      int c = Double.compare(imp1, imp2);
-      if (c == 0) {
-        c = compareRels(
-            match1.rels,
-            match2.rels);
+      c = Double.compare(imp1, imp2);
+      if (c != 0) {
+        return -c;
       }
-      return -c;
+      return -RelNodes.compareRels(match1.rels, match2.rels);
     }
   }
 

core/src/main/java/org/apache/calcite/rel/RelNodes.java

@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel;
+
+import org.apache.calcite.runtime.Utilities;
+
+import com.google.common.collect.Ordering;
+
+import java.util.Comparator;
+
+/**
+ * Utilities concerning relational expressions.
+ */
+public class RelNodes {
+  /** Comparator that provides an arbitrary but stable ordering to
+   * {@link RelNode}s. */
+  public static final Comparator<RelNode> COMPARATOR =
+      new RelNodeComparator();
+
+  /** Ordering for {@link RelNode}s. */
+  public static final Ordering<RelNode> ORDERING = Ordering.from(COMPARATOR);
+
+  private RelNodes() {}
+
+  /** Compares arrays of {@link RelNode}. */
+  public static int compareRels(RelNode[] rels0, RelNode[] rels1) {
+    int c = Utilities.compare(rels0.length, rels1.length);
+    if (c != 0) {
+      return c;
+    }
+    for (int i = 0; i < rels0.length; i++) {
+      c = COMPARATOR.compare(rels0[i], rels1[i]);
+      if (c != 0) {
+        return c;
+      }
+    }
+    return 0;
+  }
+
+  /** Arbitrary stable comparator for {@link RelNode}s. */
+  private static class RelNodeComparator implements Comparator<RelNode> {
+    public int compare(RelNode o1, RelNode o2) {
+      // Compare on field count first. It is more stable than id (when rules
+      // are added to the set of active rules).
+      final int c = Utilities.compare(o1.getRowType().getFieldCount(),
+          o2.getRowType().getFieldCount());
+      if (c != 0) {
+        return -c;
+      }
+      return Utilities.compare(o1.getId(), o2.getId());
+    }
+  }
+}
+
+// End RelNodes.java

core/src/main/java/org/apache/calcite/rel/rules/AggregateReduceFunctionsRule.java

@@ -191,11 +191,6 @@ private void reduceAggs(
             oldAggRel.getRowType().getFieldNames());
 
     ruleCall.transformTo(projectRel);
-    // If we old AggRel(SUM(0)) transforms to new AggRel($SUM0($0)) both will
-    // have the same cost, but we prefer new. Before we set the importance of
-    // old to 0, we were getting different results between JDK 1.7 and 1.8
-    // because of some arbitrary orderings of rels within an equivalence set.
-    ruleCall.getPlanner().setImportance(oldAggRel, 0d);
   }
 
   private RexNode reduceAgg(

core/src/test/java/org/apache/calcite/test/JdbcTest.java

@@ -1115,8 +1115,8 @@ public Object apply(CalciteConnection c) {
             + "and p.\"brand_name\" = 'Washington'")
         .explainMatches("including all attributes ",
             CalciteAssert.checkMaskedResultContains(""
-                + "EnumerableJoin(condition=[=($0, $38)], joinType=[inner]): rowcount = 7.050660528307499E8, cumulative cost = {1.0640240206183146E9 rows, 777302.0 cpu, 0.0 io}\n"
-                + "  EnumerableJoin(condition=[=($2, $8)], joinType=[inner]): rowcount = 2.0087351932499997E7, cumulative cost = {2.117504619375143E7 rows, 724261.0 cpu, 0.0 io}\n"
+                + "EnumerableJoin(condition=[=($0, $38)], joinType=[inner]): rowcount = 7.050660528307499E8, cumulative cost = {1.0640240216183146E9 rows, 777302.0 cpu, 0.0 io}\n"
+                + "  EnumerableJoin(condition=[=($2, $8)], joinType=[inner]): rowcount = 2.0087351932499997E7, cumulative cost = {2.117504719375143E7 rows, 724261.0 cpu, 0.0 io}\n"
                 + "    EnumerableTableScan(table=[[foodmart2, sales_fact_1997]]): rowcount = 86837.0, cumulative cost = {86837.0 rows, 86838.0 cpu, 0.0 io}\n"
                 + "    EnumerableCalc(expr#0..28=[{inputs}], expr#29=['San Francisco'], expr#30=[=($t9, $t29)], proj#0..28=[{exprs}], $condition=[$t30]): rowcount = 1542.1499999999999, cumulative cost = {11823.15 rows, 637423.0 cpu, 0.0 io}\n"
                 + "      EnumerableTableScan(table=[[foodmart2, customer]]): rowcount = 10281.0, cumulative cost = {10281.0 rows, 10282.0 cpu, 0.0 io}\n"
@@ -3135,8 +3135,8 @@ private static ImmutableMultimap<Class, Integer> x() {
             + "from \"hr\".\"emps\"\n"
             + "where \"deptno\" < 0")
         .explainContains(""
-            + "PLAN=EnumerableCalc(expr#0..2=[{inputs}], expr#3=[0], expr#4=[=($t0, $t3)], expr#5=[null], expr#6=[CASE($t4, $t5, $t1)], expr#7=[/($t2, $t0)], expr#8=[CAST($t7):JavaType(class java.lang.Integer)], CS=[$t0], C=[$t0], S=[$t6], A=[$t8])\n"
-            + "  EnumerableAggregate(group=[{}], CS=[COUNT()], agg#1=[$SUM0($0)], agg#2=[SUM($0)])\n"
+            + "PLAN=EnumerableCalc(expr#0..1=[{inputs}], expr#2=[0], expr#3=[=($t0, $t2)], expr#4=[null], expr#5=[CASE($t3, $t4, $t1)], expr#6=[/($t5, $t0)], expr#7=[CAST($t6):JavaType(class java.lang.Integer)], CS=[$t0], C=[$t0], S=[$t5], A=[$t7])\n"
+            + "  EnumerableAggregate(group=[{}], CS=[COUNT()], agg#1=[$SUM0($0)])\n"
             + "    EnumerableCalc(expr#0..4=[{inputs}], expr#5=[0], expr#6=[<($t1, $t5)], deptno=[$t1], $condition=[$t6])\n"
             + "      EnumerableTableScan(table=[[hr, emps]])\n")
         .returns("CS=0; C=0; S=null; A=null\n");

core/src/test/java/org/apache/calcite/test/LatticeTest.java

@@ -299,7 +299,7 @@ public Void apply(String materializationName) {
       .enableMaterializations(true)
       .explainContains(""
           + "EnumerableCalc(expr#0..3=[{inputs}], expr#4=[10], expr#5=[*($t3, $t4)], proj#0..2=[{exprs}], US=[$t5])\n"
-          + "  EnumerableAggregate(group=[{0}], C=[$SUM0($2)], Q=[MIN($1)], agg#2=[$SUM0($4)])\n"
+          + "  EnumerableAggregate(group=[{0}], C=[$SUM0($2)], Q=[MIN($1)], agg#2=[SUM($4)])\n"
           + "    EnumerableTableScan(table=[[adhoc, m{27, 31}")
       .returnsUnordered("the_year=1997; C=86837; Q=Q1; US=2667730.0000")
       .sameResultWithMaterializationsDisabled();

core/src/test/java/org/apache/calcite/test/ScannableTableTest.java

@@ -161,7 +161,7 @@ public class ScannableTableTest {
     resultSet.close();
     // Only 2 rows came out of the table. If the value is 4, it means that the
     // planner did not pass the filter down.
-    assertThat(buf.toString(), equalTo("returnCount=2, projects=[0, 1, 2]"));
+    assertThat(buf.toString(), equalTo("returnCount=2"));
     buf.setLength(0);
 
     // Now with an "uncooperative" filterable table that refuses to accept