[calcite] branch master updated: [CALCITE-4003] Disallow cross convention matching and generation in TransformationRule

[hy...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

hyuan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/master by this push:
     new 0c8d0fe  [CALCITE-4003] Disallow cross convention matching and generation in TransformationRule
0c8d0fe is described below

commit 0c8d0fe2c4b9affdd247ec1f376231e7b961a912
Author: Haisheng Yuan <h....@alibaba-inc.com>
AuthorDate: Sun May 17 11:23:10 2020 -0500

    [CALCITE-4003] Disallow cross convention matching and generation in TransformationRule
---
 .../org/apache/calcite/plan/volcano/VolcanoRuleCall.java | 16 +++++++++++++++-
 .../org/apache/calcite/rel/rules/ProjectRemoveRule.java  |  1 +
 .../org/apache/calcite/rel/rules/SortRemoveRule.java     |  3 ++-
 site/_docs/history.md                                    |  2 ++
 4 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoRuleCall.java b/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoRuleCall.java
index 189a47a..bbc32b5 100644
--- a/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoRuleCall.java
+++ b/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoRuleCall.java
@@ -21,8 +21,10 @@ import org.apache.calcite.plan.RelOptListener;
 import org.apache.calcite.plan.RelOptRuleCall;
 import org.apache.calcite.plan.RelOptRuleOperand;
 import org.apache.calcite.plan.RelOptRuleOperandChildPolicy;
+import org.apache.calcite.rel.PhysicalNode;
 import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.rules.SubstitutionRule;
+import org.apache.calcite.rel.rules.TransformationRule;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -92,6 +94,12 @@ public class VolcanoRuleCall extends RelOptRuleCall {
   // implement RelOptRuleCall
   public void transformTo(RelNode rel, Map<RelNode, RelNode> equiv,
       RelHintsPropagator handler) {
+    if (rel instanceof PhysicalNode
+        && rule instanceof TransformationRule) {
+      throw new RuntimeException(
+          rel + " is a PhysicalNode, which is not allowed in " + rule);
+    }
+
     rel = handler.propagate(rels[0], rel);
     if (LOGGER.isDebugEnabled()) {
       LOGGER.debug("Transform to: rel#{} via {}{}", rel.getId(), getRule(),
@@ -136,7 +144,9 @@ public class VolcanoRuleCall extends RelOptRuleCall {
         volcanoPlanner.ensureRegistered(
             entry.getKey(), entry.getValue());
       }
-      volcanoPlanner.ensureRegistered(rel, rels[0]);
+      // The subset is not used, but we need it, just for debugging
+      //noinspection unused
+      RelSubset subset = volcanoPlanner.ensureRegistered(rel, rels[0]);
       rels[0].getCluster().invalidateMetadataQuery();
 
       if (volcanoPlanner.getListener() != null) {
@@ -349,6 +359,10 @@ public class VolcanoRuleCall extends RelOptRuleCall {
       }
 
       for (RelNode rel : successors) {
+        if (operand.getRule() instanceof TransformationRule
+            && rel.getConvention() != previous.getConvention()) {
+          continue;
+        }
         if (!operand.matches(rel)) {
           continue;
         }
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
index ba8ff2e..b1e935b 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/ProjectRemoveRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/ProjectRemoveRule.java
@@ -66,6 +66,7 @@ public class ProjectRemoveRule extends RelOptRule implements SubstitutionRule {
           childProject.getInput(), childProject.getProjects(),
           project.getRowType());
     }
+    stripped = convert(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
index 860e666..1d71ea4 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/SortRemoveRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/SortRemoveRule.java
@@ -61,7 +61,8 @@ public class SortRemoveRule extends RelOptRule implements TransformationRule {
     final RelCollation collation = sort.getCollation();
     assert collation == sort.getTraitSet()
         .getTrait(RelCollationTraitDef.INSTANCE);
-    final RelTraitSet traits = sort.getInput().getTraitSet().replace(collation);
+    final RelTraitSet traits = sort.getInput().getTraitSet()
+        .replace(collation).replace(sort.getConvention());
     call.transformTo(convert(sort.getInput(), traits));
   }
 }
diff --git a/site/_docs/history.md b/site/_docs/history.md
index b769b48..f21942f 100644
--- a/site/_docs/history.md
+++ b/site/_docs/history.md
@@ -34,6 +34,8 @@ Downloads are available on the
 
 * [<a href="https://issues.apache.org/jira/browse/CALCITE-4032">CALCITE-4032</a>]
   Mark `CalcMergeRule` as `TransformationRule`
+* [<a href="https://issues.apache.org/jira/browse/CALCITE-4003">CALCITE-4003</a>]
+  Disallow cross convention matching and `PhysicalNode` generation in `TransformationRule`
 
 ## <a href="https://github.com/apache/calcite/releases/tag/calcite-1.23.0">1.23.0</a> / 2020-05-23
 {: #v1-23-0}