[drill] branch master updated: DRILL-6212: Prevent recursive cast expressions

[ti...@apache.org]

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

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


The following commit(s) were added to refs/heads/master by this push:
     new b447260  DRILL-6212: Prevent recursive cast expressions
b447260 is described below

commit b447260e49dc4a8c906f5b310c037fe6dd77166f
Author: chunhui-shi <cs...@maprtech.com>
AuthorDate: Wed Jun 6 12:25:31 2018 -0700

    DRILL-6212: Prevent recursive cast expressions
    
    closes #1319
---
 .../planner/logical/DrillMergeProjectRule.java     | 113 ++++++++++++++++++---
 .../org/apache/drill/TestProjectWithFunctions.java |  54 ++++++++++
 .../src/test/resources/view/emp_6212.view.drill    |  10 ++
 3 files changed, 165 insertions(+), 12 deletions(-)

diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillMergeProjectRule.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillMergeProjectRule.java
index 94964ef..6c151ed 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillMergeProjectRule.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillMergeProjectRule.java
@@ -17,22 +17,37 @@
  */
 package org.apache.drill.exec.planner.logical;
 
-
-import org.apache.calcite.plan.Convention;
-import org.apache.calcite.plan.ConventionTraitDef;
+import org.apache.calcite.plan.RelOptRule;
 import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.rel.RelNode;
 import org.apache.calcite.rel.core.Project;
-import org.apache.calcite.rel.rules.ProjectMergeRule;
+import org.apache.calcite.rel.logical.LogicalProject;
+import org.apache.calcite.rex.RexUtil;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.calcite.util.Permutation;
 import org.apache.drill.exec.expr.fn.FunctionImplementationRegistry;
 import org.apache.calcite.rel.core.RelFactories.ProjectFactory;
 import org.apache.calcite.rex.RexCall;
 import org.apache.calcite.rex.RexNode;
 import org.apache.drill.exec.planner.DrillRelBuilder;
 
-public class DrillMergeProjectRule extends ProjectMergeRule {
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Rule for merging two projects provided the projects aren't projecting identical sets of
+ * input references.
+ *
+ * NOTE: This rules does not extend the Calcite ProjectMergeRule
+ * because of CALCITE-2223. Once, fixed this rule be changed accordingly. Please see DRILL-6501.
+ */
+public class DrillMergeProjectRule extends RelOptRule {
 
   private FunctionImplementationRegistry functionRegistry;
   private static DrillMergeProjectRule INSTANCE = null;
+  private final boolean force;
 
   public static DrillMergeProjectRule getInstance(boolean force, ProjectFactory pFactory,
       FunctionImplementationRegistry functionRegistry) {
@@ -44,7 +59,12 @@ public class DrillMergeProjectRule extends ProjectMergeRule {
 
   private DrillMergeProjectRule(boolean force, ProjectFactory pFactory,
       FunctionImplementationRegistry functionRegistry) {
-    super(force, DrillRelBuilder.proto(pFactory));
+
+    super(operand(LogicalProject.class,
+        operand(LogicalProject.class, any())),
+        DrillRelBuilder.proto(pFactory),
+        "DrillMergeProjectRule" + (force ? ":force_mode" : ""));
+    this.force = force;
     this.functionRegistry = functionRegistry;
   }
 
@@ -53,12 +73,6 @@ public class DrillMergeProjectRule extends ProjectMergeRule {
     Project topProject = call.rel(0);
     Project bottomProject = call.rel(1);
 
-    // Make sure both projects be LogicalProject.
-    if (topProject.getTraitSet().getTrait(ConventionTraitDef.INSTANCE) != Convention.NONE ||
-        bottomProject.getTraitSet().getTrait(ConventionTraitDef.INSTANCE) != Convention.NONE) {
-      return false;
-    }
-
     // We have a complex output type do not fire the merge project rule
     if (checkComplexOutput(topProject) || checkComplexOutput(bottomProject)) {
       return false;
@@ -77,4 +91,79 @@ public class DrillMergeProjectRule extends ProjectMergeRule {
     }
     return false;
   }
+
+  @Override
+  public void onMatch(RelOptRuleCall call) {
+    final Project topProject = call.rel(0);
+    final Project bottomProject = call.rel(1);
+    final RelBuilder relBuilder = call.builder();
+
+    // If one or both projects are permutations, short-circuit the complex logic
+    // of building a RexProgram.
+    final Permutation topPermutation = topProject.getPermutation();
+    if (topPermutation != null) {
+      if (topPermutation.isIdentity()) {
+        // Let ProjectRemoveRule handle this.
+        return;
+      }
+      final Permutation bottomPermutation = bottomProject.getPermutation();
+      if (bottomPermutation != null) {
+        if (bottomPermutation.isIdentity()) {
+          // Let ProjectRemoveRule handle this.
+          return;
+        }
+        final Permutation product = topPermutation.product(bottomPermutation);
+        relBuilder.push(bottomProject.getInput());
+        relBuilder.project(relBuilder.fields(product),
+            topProject.getRowType().getFieldNames());
+        call.transformTo(relBuilder.build());
+        return;
+      }
+    }
+
+    // If we're not in force mode and the two projects reference identical
+    // inputs, then return and let ProjectRemoveRule replace the projects.
+    if (!force) {
+      if (RexUtil.isIdentity(topProject.getProjects(),
+          topProject.getInput().getRowType())) {
+        return;
+      }
+    }
+
+    final List<RexNode> pushedProjects =
+        RelOptUtil.pushPastProject(topProject.getProjects(), bottomProject);
+    final List<RexNode> newProjects = simplifyCast(pushedProjects);
+    final RelNode input = bottomProject.getInput();
+    if (RexUtil.isIdentity(newProjects, input.getRowType())) {
+      if (force
+          || input.getRowType().getFieldNames()
+          .equals(topProject.getRowType().getFieldNames())) {
+        call.transformTo(input);
+        return;
+      }
+    }
+
+    // replace the two projects with a combined projection
+    relBuilder.push(bottomProject.getInput());
+    relBuilder.project(newProjects, topProject.getRowType().getFieldNames());
+    call.transformTo(relBuilder.build());
+  }
+
+  public static List<RexNode> simplifyCast(List<RexNode> projectExprs) {
+    final List<RexNode> list = new ArrayList<>();
+    for (RexNode rex: projectExprs) {
+      if (rex.getKind() == SqlKind.CAST) {
+        RexNode operand = ((RexCall) rex).getOperands().get(0);
+        while (operand.getKind() == SqlKind.CAST
+            && operand.getType().equals(rex.getType())) {
+          rex = operand;
+          operand = ((RexCall) rex).getOperands().get(0);
+        }
+
+      }
+      list.add(rex);
+    }
+    return list;
+  }
+
 }
diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestProjectWithFunctions.java b/exec/java-exec/src/test/java/org/apache/drill/TestProjectWithFunctions.java
new file mode 100644
index 0000000..58cbad8
--- /dev/null
+++ b/exec/java-exec/src/test/java/org/apache/drill/TestProjectWithFunctions.java
@@ -0,0 +1,54 @@
+/*
+ * 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.drill;
+
+import org.apache.drill.categories.PlannerTest;
+import org.apache.drill.test.ClusterFixture;
+import org.apache.drill.test.ClusterFixtureBuilder;
+import org.apache.drill.test.ClusterTest;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.nio.file.Paths;
+
+/**
+ * Test the optimizer plan in terms of projecting different functions e.g. cast
+ */
+@Category(PlannerTest.class)
+public class TestProjectWithFunctions extends ClusterTest {
+
+  @BeforeClass
+  public static void setupFiles() {
+    dirTestWatcher.copyResourceToRoot(Paths.get("view"));
+  }
+
+  @Before
+  public void setup() throws Exception {
+    ClusterFixtureBuilder builder = ClusterFixture.builder(dirTestWatcher);
+    startCluster(builder);
+  }
+
+  @Test
+  public void testCastFunctions() throws Exception {
+    String sql = "select t1.f from dfs.`view/emp_6212.view.drill` as t inner join dfs.`view/emp_6212.view.drill` as t1 " +
+        "on cast(t.f as int) = cast(t1.f as int) and cast(t.f as int) = 10 and cast(t1.f as int) = 10";
+    client.queryBuilder().sql(sql).run();
+  }
+}
diff --git a/exec/java-exec/src/test/resources/view/emp_6212.view.drill b/exec/java-exec/src/test/resources/view/emp_6212.view.drill
new file mode 100644
index 0000000..72dc5b0
--- /dev/null
+++ b/exec/java-exec/src/test/resources/view/emp_6212.view.drill
@@ -0,0 +1,10 @@
+{
+  "name" : "emp_6212",
+  "sql" : "SELECT CAST(`department_id` AS INTEGER) AS `f`\nFROM `cp`.`employee.json`",
+  "fields" : [ {
+    "name" : "f",
+    "type" : "INTEGER",
+    "isNullable" : true
+  } ],
+  "workspaceSchemaPath" : [ "dfs", "tmp" ]
+}
\ No newline at end of file

-- 
To stop receiving notification emails like this one, please contact
timothyfarkas@apache.org.