Skip to content
Permalink
Browse files
DRILL-6212: Prevent recursive cast expressions
closes #1319
  • Loading branch information
chunhui-shi authored and ilooner committed Jun 17, 2018
1 parent 4baf769 commit b447260e49dc4a8c906f5b310c037fe6dd77166f
Showing 3 changed files with 165 additions and 12 deletions.
@@ -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;
}

}
@@ -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();
}
}
@@ -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" ]
}

0 comments on commit b447260

Please sign in to comment.