You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by da...@apache.org on 2019/11/30 08:25:59 UTC

[calcite] branch master updated: [CALCITE-3353] ProjectJoinTransposeRule caused AssertionError when creating a new Join (Wenhui Tang)

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

danny0405 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 bb18888  [CALCITE-3353] ProjectJoinTransposeRule caused AssertionError when creating a new Join (Wenhui Tang)
bb18888 is described below

commit bb18888c43a3943f5c1265bab5d87830633c465b
Author: wenhuitang <we...@yeah.net>
AuthorDate: Wed Sep 18 21:32:12 2019 +0800

    [CALCITE-3353] ProjectJoinTransposeRule caused AssertionError when creating a new Join (Wenhui Tang)
    
    * Change ProjectJoinTransposeRule.INSTANCE to match only logical nodes.
    * User can use the constructor directly to make the rule instance if
      they want to match convention nodes.
    
    close apache/calcite#1462
---
 .../java/org/apache/calcite/plan/RelOptUtil.java   | 23 ++++++++
 .../rel/rules/ProjectJoinTransposeRule.java        | 61 +++++++++++++++++++---
 .../org/apache/calcite/test/RelOptRulesTest.java   | 55 ++++++++++++++++++-
 3 files changed, 130 insertions(+), 9 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
index 042c0f6..9daf30d 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
@@ -462,6 +462,29 @@ public abstract class RelOptUtil {
     return mapping;
   }
 
+  /**
+   * Returns a permutation describing where the Project's fields come from
+   * after the Project is pushed down.
+   */
+  public static Mappings.TargetMapping permutationPushDownProject(
+      List<RexNode> nodes,
+      RelDataType inputRowType,
+      int sourceOffset,
+      int targetOffset) {
+    final Mappings.TargetMapping mapping =
+        Mappings.create(MappingType.PARTIAL_FUNCTION,
+            inputRowType.getFieldCount() + sourceOffset,
+            nodes.size() + targetOffset);
+    for (Ord<RexNode> node : Ord.zip(nodes)) {
+      if (node.e instanceof RexInputRef) {
+        mapping.set(
+            ((RexInputRef) node.e).getIndex() + sourceOffset,
+            node.i + targetOffset);
+      }
+    }
+    return mapping;
+  }
+
   @Deprecated // to be removed before 2.0
   public static RelNode createExistsPlan(
       RelOptCluster cluster,
diff --git a/core/src/main/java/org/apache/calcite/rel/rules/ProjectJoinTransposeRule.java b/core/src/main/java/org/apache/calcite/rel/rules/ProjectJoinTransposeRule.java
index 7c98114..35821a7 100644
--- a/core/src/main/java/org/apache/calcite/rel/rules/ProjectJoinTransposeRule.java
+++ b/core/src/main/java/org/apache/calcite/rel/rules/ProjectJoinTransposeRule.java
@@ -19,16 +19,25 @@ package org.apache.calcite.rel.rules;
 import org.apache.calcite.plan.RelOptRule;
 import org.apache.calcite.plan.RelOptRuleCall;
 import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.rel.RelCollation;
+import org.apache.calcite.rel.RelCollationTraitDef;
+import org.apache.calcite.rel.RelCollations;
+import org.apache.calcite.rel.RelFieldCollation;
 import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.core.Join;
 import org.apache.calcite.rel.core.Project;
 import org.apache.calcite.rel.core.RelFactories;
+import org.apache.calcite.rel.logical.LogicalJoin;
+import org.apache.calcite.rel.logical.LogicalProject;
 import org.apache.calcite.rel.type.RelDataTypeField;
 import org.apache.calcite.rex.RexCall;
 import org.apache.calcite.rex.RexNode;
 import org.apache.calcite.rex.RexOver;
 import org.apache.calcite.rex.RexShuttle;
+import org.apache.calcite.rex.RexUtil;
 import org.apache.calcite.tools.RelBuilderFactory;
+import org.apache.calcite.util.mapping.Mappings;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -40,8 +49,17 @@ import java.util.List;
  * the join.
  */
 public class ProjectJoinTransposeRule extends RelOptRule {
+  /**
+   * A instance for ProjectJoinTransposeRule that pushes a
+   * {@link org.apache.calcite.rel.logical.LogicalProject}
+   * past a {@link org.apache.calcite.rel.logical.LogicalJoin}
+   * by splitting the projection into a projection on top of each child of
+   * the join.
+   */
   public static final ProjectJoinTransposeRule INSTANCE =
-      new ProjectJoinTransposeRule(expr -> !(expr instanceof RexOver),
+      new ProjectJoinTransposeRule(
+          LogicalProject.class, LogicalJoin.class,
+          expr -> !(expr instanceof RexOver),
           RelFactories.LOGICAL_BUILDER);
 
   //~ Instance fields --------------------------------------------------------
@@ -57,15 +75,14 @@ public class ProjectJoinTransposeRule extends RelOptRule {
    * Creates a ProjectJoinTransposeRule with an explicit condition.
    *
    * @param preserveExprCondition Condition for expressions that should be
-   *                              preserved in the projection
+   *                             preserved in the projection
    */
   public ProjectJoinTransposeRule(
+      Class<? extends Project> projectClass,
+      Class<? extends Join> joinClass,
       PushProjector.ExprCondition preserveExprCondition,
       RelBuilderFactory relFactory) {
-    super(
-        operand(Project.class,
-            operand(Join.class, any())),
-        relFactory, null);
+    super(operand(projectClass, operand(joinClass, any())), relFactory, null);
     this.preserveExprCondition = preserveExprCondition;
   }
 
@@ -139,11 +156,39 @@ public class ProjectJoinTransposeRule extends RelOptRule {
               projJoinFieldList,
               adjustments);
     }
-
+    RelTraitSet traits = join.getTraitSet();
+    final List<RelCollation> originCollations = traits.getTraits(RelCollationTraitDef.INSTANCE);
+
+    if (originCollations != null && !originCollations.isEmpty()) {
+      List<RelCollation> newCollations = new ArrayList<>();
+      final int originLeftCnt = join.getLeft().getRowType().getFieldCount();
+      final Mappings.TargetMapping leftMapping = RelOptUtil.permutationPushDownProject(
+              ((Project) leftProjRel).getProjects(), join.getLeft().getRowType(),
+              0, 0);
+      final Mappings.TargetMapping rightMapping = RelOptUtil.permutationPushDownProject(
+              ((Project) rightProjRel).getProjects(), join.getRight().getRowType(),
+              originLeftCnt, leftProjRel.getRowType().getFieldCount());
+      for (RelCollation collation: originCollations) {
+        List<RelFieldCollation> fc = new ArrayList<>();
+        final List<RelFieldCollation> fieldCollations = collation.getFieldCollations();
+        for (RelFieldCollation relFieldCollation: fieldCollations) {
+          final int fieldIndex = relFieldCollation.getFieldIndex();
+          if (fieldIndex < originLeftCnt) {
+            fc.add(RexUtil.apply(leftMapping, relFieldCollation));
+          } else {
+            fc.add(RexUtil.apply(rightMapping, relFieldCollation));
+          }
+        }
+        newCollations.add(RelCollations.of(fc));
+      }
+      if (!newCollations.isEmpty()) {
+        traits = traits.replace(newCollations);
+      }
+    }
     // create a new join with the projected children
     Join newJoinRel =
         join.copy(
-            join.getTraitSet(),
+            traits,
             newJoinFilter,
             leftProjRel,
             rightProjRel,
diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
index 3ecadab..2f11a26 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -47,6 +47,7 @@ import org.apache.calcite.rel.core.Aggregate;
 import org.apache.calcite.rel.core.CorrelationId;
 import org.apache.calcite.rel.core.Filter;
 import org.apache.calcite.rel.core.Intersect;
+import org.apache.calcite.rel.core.Join;
 import org.apache.calcite.rel.core.JoinRelType;
 import org.apache.calcite.rel.core.Minus;
 import org.apache.calcite.rel.core.Project;
@@ -131,6 +132,7 @@ import org.apache.calcite.rel.type.RelDataTypeFactory;
 import org.apache.calcite.rex.RexCall;
 import org.apache.calcite.rex.RexInputRef;
 import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexOver;
 import org.apache.calcite.runtime.Hook;
 import org.apache.calcite.sql.SqlFunction;
 import org.apache.calcite.sql.SqlFunctionCategory;
@@ -140,6 +142,7 @@ import org.apache.calcite.sql.SqlOperator;
 import org.apache.calcite.sql.SqlOperatorBinding;
 import org.apache.calcite.sql.SqlSpecialOperator;
 import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.parser.SqlParser;
 import org.apache.calcite.sql.type.OperandTypes;
 import org.apache.calcite.sql.type.ReturnTypes;
 import org.apache.calcite.sql.type.SqlTypeName;
@@ -148,6 +151,8 @@ import org.apache.calcite.sql.validate.SqlValidator;
 import org.apache.calcite.sql2rel.RelDecorrelator;
 import org.apache.calcite.sql2rel.SqlToRelConverter;
 import org.apache.calcite.test.catalog.MockCatalogReader;
+import org.apache.calcite.tools.FrameworkConfig;
+import org.apache.calcite.tools.Frameworks;
 import org.apache.calcite.tools.Program;
 import org.apache.calcite.tools.Programs;
 import org.apache.calcite.tools.RelBuilder;
@@ -875,6 +880,53 @@ public class RelOptRulesTest extends RelOptTestBase {
         .check();
   }
 
+
+  /**
+   * Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-3353">[CALCITE-3353]
+   * ProjectJoinTransposeRule caused AssertionError when creating a new Join</a>.
+   */
+  @Test
+  public void testProjectJoinTransposeWithMergeJoin() {
+    ProjectJoinTransposeRule testRule = new ProjectJoinTransposeRule(
+            Project.class, Join.class, expr -> !(expr instanceof RexOver),
+            RelFactories.LOGICAL_BUILDER);
+    ImmutableList<RelOptRule> commonRules = ImmutableList.of(
+            EnumerableRules.ENUMERABLE_PROJECT_RULE,
+            EnumerableRules.ENUMERABLE_MERGE_JOIN_RULE,
+            EnumerableRules.ENUMERABLE_SORT_RULE,
+            EnumerableRules.ENUMERABLE_VALUES_RULE);
+    final RuleSet rules = RuleSets.ofList(ImmutableList.<RelOptRule>builder()
+            .addAll(commonRules)
+            .add(ProjectJoinTransposeRule.INSTANCE)
+            .build());
+    final RuleSet testRules = RuleSets.ofList(ImmutableList.<RelOptRule>builder()
+            .addAll(commonRules)
+            .add(testRule).build());
+
+    FrameworkConfig config = Frameworks.newConfigBuilder()
+            .parserConfig(SqlParser.Config.DEFAULT)
+            .traitDefs(ConventionTraitDef.INSTANCE, RelCollationTraitDef.INSTANCE)
+            .build();
+
+    RelBuilder builder = RelBuilder.create(config);
+    RelNode logicalPlan = builder
+            .values(new String[]{"id", "name"}, "1", "anna", "2", "bob", "3", "tom")
+            .values(new String[]{"name", "age"}, "anna", "14", "bob", "17", "tom", "22")
+            .join(JoinRelType.INNER, "name")
+            .project(builder.field(3))
+            .build();
+
+    RelTraitSet desiredTraits = logicalPlan.getTraitSet()
+            .replace(EnumerableConvention.INSTANCE);
+    RelOptPlanner planner = logicalPlan.getCluster().getPlanner();
+    RelNode enumerablePlan1 = Programs.of(rules).run(planner, logicalPlan,
+            desiredTraits, ImmutableList.of(), ImmutableList.of());
+    RelNode enumerablePlan2 = Programs.of(testRules).run(planner, logicalPlan,
+            desiredTraits, ImmutableList.of(), ImmutableList.of());
+    assertEquals(RelOptUtil.toString(enumerablePlan1), RelOptUtil.toString(enumerablePlan2));
+  }
+
   /** Test case for
    * <a href="https://issues.apache.org/jira/browse/CALCITE-889">[CALCITE-889]
    * Implement SortUnionTransposeRule</a>. */
@@ -6352,7 +6404,8 @@ public class RelOptRulesTest extends RelOptTestBase {
 
   @Test public void testProjectJoinTransposeItem() {
     ProjectJoinTransposeRule projectJoinTransposeRule =
-        new ProjectJoinTransposeRule(skipItem, RelFactories.LOGICAL_BUILDER);
+        new ProjectJoinTransposeRule(Project.class, Join.class, skipItem, RelFactories
+          .LOGICAL_BUILDER);
 
     String query = "select t1.c_nationkey[0], t2.c_nationkey[0] "
         + "from sales.customer as t1 left outer join sales.customer as t2 "