[
https://issues.apache.org/jira/browse/DRILL-6212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16515173#comment-16515173
]
ASF GitHub Bot commented on DRILL-6212:
---------------------------------------
ilooner closed pull request #1319: DRILL-6212: Prevent recursive cast
expressions
URL: https://github.com/apache/drill/pull/1319
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 94964efd1c..6c151ed7ea 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 static DrillMergeProjectRule getInstance(boolean
force, ProjectFactory pF
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 boolean matches(RelOptRuleCall call) {
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 @@ private boolean checkComplexOutput(Project project) {
}
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 0000000000..58cbad8ccf
--- /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 0000000000..72dc5b006c
--- /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
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> A simple join is recursing too deep in planning and eventually throwing stack
> overflow.
> ---------------------------------------------------------------------------------------
>
> Key: DRILL-6212
> URL: https://issues.apache.org/jira/browse/DRILL-6212
> Project: Apache Drill
> Issue Type: Bug
> Components: Query Planning & Optimization
> Affects Versions: 1.12.0
> Reporter: Hanumath Rao Maduri
> Assignee: Gautam Kumar Parai
> Priority: Critical
> Labels: ready-to-commit
> Fix For: 1.14.0
>
>
> Create two views using following statements.
> {code}
> create view v1 as select cast(greeting as int) f from
> dfs.`/home/mapr/data/json/temp.json`;
> create view v2 as select cast(greeting as int) f from
> dfs.`/home/mapr/data/json/temp.json`;
> {code}
> Executing the following join query produces a stack overflow during the
> planning phase.
> {code}
> select t1.f from dfs.tmp.v1 as t inner join dfs.tmp.v2 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;
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
