[CALCITE-5401] Rule fired by HepPlanner can return Volcano's RelSubset

zabetak · Jun 9, 2023 · 2e3e4ae · 2e3e4ae
1 parent 9f41a12
commit 2e3e4ae
Show file tree

Hide file tree

Showing 11 changed files with 41 additions and 11 deletions.
diff --git a/...rc/main/java/org/apache/calcite/adapter/enumerable/EnumerableBatchNestedLoopJoinRule.java b/...rc/main/java/org/apache/calcite/adapter/enumerable/EnumerableBatchNestedLoopJoinRule.java
@@ -140,9 +140,9 @@ public EnumerableBatchNestedLoopJoinRule(RelBuilderFactory relBuilderFactory,
 
     call.transformTo(
         EnumerableBatchNestedLoopJoin.create(
-            convert(join.getLeft(), join.getLeft().getTraitSet()
+            convert(call.getPlanner(), join.getLeft(), join.getLeft().getTraitSet()
                 .replace(EnumerableConvention.INSTANCE)),
-            convert(right, right.getTraitSet()
+            convert(call.getPlanner(), right, right.getTraitSet()
                 .replace(EnumerableConvention.INSTANCE)),
             join.getCondition(),
             requiredColumns.build(),

diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableLimitRule.java b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableLimitRule.java
@@ -57,7 +57,8 @@ protected EnumerableLimitRule(Config config) {
     }
     call.transformTo(
         EnumerableLimit.create(
-            convert(input, input.getTraitSet().replace(EnumerableConvention.INSTANCE)),
+            convert(call.getPlanner(), input,
+                input.getTraitSet().replace(EnumerableConvention.INSTANCE)),
             sort.offset,
             sort.fetch));
   }

diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableLimitSortRule.java b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableLimitSortRule.java
@@ -43,7 +43,7 @@ public EnumerableLimitSortRule(Config config) {
     RelNode input = sort.getInput();
     final Sort o =
         EnumerableLimitSort.create(
-            convert(input,
+            convert(call.getPlanner(), input,
                 input.getTraitSet().replace(EnumerableConvention.INSTANCE)),
             sort.getCollation(), sort.offset, sort.fetch);
 

diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeUnionRule.java b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeUnionRule.java
@@ -135,7 +135,8 @@ public EnumerableMergeUnionRule(Config config) {
       final RelNode newInput =
           sort.copy(sort.getTraitSet(), unsortedInput, collation, null, inputFetch);
       inputs.add(
-          convert(newInput, newInput.getTraitSet().replace(EnumerableConvention.INSTANCE)));
+          convert(call.getPlanner(), newInput,
+              newInput.getTraitSet().replace(EnumerableConvention.INSTANCE)));
     }
 
     RelNode result = EnumerableMergeUnion.create(sort.getCollation(), inputs, union.all);

diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptRule.java b/core/src/main/java/org/apache/calcite/plan/RelOptRule.java
@@ -588,8 +588,10 @@ public boolean matches(RelOptRuleCall call) {
    * @return a relational expression with the desired traits; never null
    */
   public static RelNode convert(RelNode rel, RelTraitSet toTraits) {
-    RelOptPlanner planner = rel.getCluster().getPlanner();
+    return convert(rel.getCluster().getPlanner(), rel, toTraits);
+  }
 
+  public static RelNode convert(RelOptPlanner planner, RelNode rel, RelTraitSet toTraits) {
     RelTraitSet outTraits = rel.getTraitSet();
     for (int i = 0; i < toTraits.size(); i++) {
       RelTrait toTrait = toTraits.getTrait(i);
@@ -614,7 +616,10 @@ public static RelNode convert(RelNode rel, RelTraitSet toTraits) {
    * @return a relational expression with the desired trait; never null
    */
   public static RelNode convert(RelNode rel, @Nullable RelTrait toTrait) {
-    RelOptPlanner planner = rel.getCluster().getPlanner();
+    return convert(rel.getCluster().getPlanner(), rel, toTrait);
+  }
+
+  public static RelNode convert(RelOptPlanner planner, RelNode rel, @Nullable RelTrait toTrait) {
     RelTraitSet outTraits = rel.getTraitSet();
     if (toTrait != null) {
       outTraits = outTraits.replace(toTrait);

diff --git a/core/src/main/java/org/apache/calcite/rel/rules/AggregateRemoveRule.java b/core/src/main/java/org/apache/calcite/rel/rules/AggregateRemoveRule.java
@@ -147,7 +147,7 @@ public static boolean canFlattenStatic(AggregateCall aggregateCall) {
       projects.add(cast);
     }
 
-    final RelNode newInput = convert(input, aggregate.getTraitSet().simplify());
+    final RelNode newInput = convert(call.getPlanner(), input, aggregate.getTraitSet().simplify());
     relBuilder.push(newInput);
     if (!projects.isEmpty()) {
       projects.addAll(0, relBuilder.fields(aggregate.getGroupSet()));

diff --git a/core/src/main/java/org/apache/calcite/rel/rules/CalcRemoveRule.java b/core/src/main/java/org/apache/calcite/rel/rules/CalcRemoveRule.java
@@ -59,6 +59,7 @@ public CalcRemoveRule(RelBuilderFactory relBuilderFactory) {
     input = call.getPlanner().register(input, calc);
     call.transformTo(
         convert(
+            call.getPlanner(),
             input,
             calc.getTraitSet()));
   }

diff --git a/core/src/main/java/org/apache/calcite/rel/rules/ProjectRemoveRule.java b/core/src/main/java/org/apache/calcite/rel/rules/ProjectRemoveRule.java
@@ -67,7 +67,7 @@ public ProjectRemoveRule(RelBuilderFactory relBuilderFactory) {
               childProject.getInput(), childProject.getProjects(),
               project.getRowType());
     }
-    stripped = convert(stripped, project.getConvention());
+    stripped = convert(call.getPlanner(), stripped, project.getConvention());
     call.transformTo(stripped);
   }
 

diff --git a/core/src/main/java/org/apache/calcite/rel/rules/SortRemoveRule.java b/core/src/main/java/org/apache/calcite/rel/rules/SortRemoveRule.java
@@ -70,7 +70,7 @@ public SortRemoveRule(RelBuilderFactory relBuilderFactory) {
         .getTrait(RelCollationTraitDef.INSTANCE);
     final RelTraitSet traits = sort.getInput().getTraitSet()
         .replace(collation).replaceIf(ConventionTraitDef.INSTANCE, sort::getConvention);
-    call.transformTo(convert(sort.getInput(), traits));
+    call.transformTo(convert(call.getPlanner(), sort.getInput(), traits));
   }
 
   /** Rule configuration. */

diff --git a/core/src/test/java/org/apache/calcite/test/HepPlannerTest.java b/core/src/test/java/org/apache/calcite/test/HepPlannerTest.java
@@ -26,9 +26,11 @@
 import org.apache.calcite.rel.externalize.RelDotWriter;
 import org.apache.calcite.rel.logical.LogicalIntersect;
 import org.apache.calcite.rel.logical.LogicalUnion;
+import org.apache.calcite.rel.logical.LogicalValues;
 import org.apache.calcite.rel.rules.CoerceInputsRule;
 import org.apache.calcite.rel.rules.CoreRules;
 import org.apache.calcite.sql.SqlExplainLevel;
+import org.apache.calcite.tools.RelBuilder;
 
 import com.google.common.collect.ImmutableList;
 
@@ -41,6 +43,7 @@
 
 import static org.apache.calcite.test.Matchers.isLinux;
 
+import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -421,4 +424,23 @@ long getApplyTimes() {
     @Override public void relChosen(RelChosenEvent event) {
     }
   }
+
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-5401">[CALCITE-5401]
+   * Rule fired by HepPlanner can return Volcano's RelSubset</a>. */
+  @Test void testAggregateRemove() {
+    final RelBuilder builder = RelBuilderTest.createBuilder(c -> c.withAggregateUnique(true));
+    final RelNode root =
+        builder
+            .values(new String[]{"i"}, 1, 2, 3)
+            .distinct()
+            .build();
+    final HepProgram program = new HepProgramBuilder()
+        .addRuleInstance(CoreRules.AGGREGATE_REMOVE)
+        .build();
+    final HepPlanner planner = new HepPlanner(program);
+    planner.setRoot(root);
+    final RelNode result = planner.findBestExp();
+    assertThat(result, is(instanceOf(LogicalValues.class)));
+  }
 }
diff --git a/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeRules.java b/geode/src/main/java/org/apache/calcite/adapter/geode/rel/GeodeRules.java
@@ -225,7 +225,7 @@ protected GeodeSortLimitRule(GeodeSortLimitRuleConfig config) {
 
       GeodeSort geodeSort =
           new GeodeSort(sort.getCluster(), traitSet,
-              convert(sort.getInput(), traitSet.replace(RelCollations.EMPTY)),
+              convert(call.getPlanner(), sort.getInput(), traitSet.replace(RelCollations.EMPTY)),
               sort.getCollation(), sort.fetch);
 
       call.transformTo(geodeSort);