drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j..@apache.org
Subject [1/4] drill git commit: DRILL-5263: Prevent left NLJoin with non scalar subqueries
Date Tue, 21 Feb 2017 15:33:31 GMT
Repository: drill
Updated Branches:
  refs/heads/master 300e9349a -> 38f816a45


DRILL-5263: Prevent left NLJoin with non scalar subqueries


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/3ba24fb0
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/3ba24fb0
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/3ba24fb0

Branch: refs/heads/master
Commit: 3ba24fb05aa5365baf8e44a9dd437dc57f46d9b9
Parents: 300e934
Author: Serhii-Harnyk <serhii.harnyk@gmail.com>
Authored: Mon Feb 13 14:30:27 2017 +0000
Committer: Jinfeng Ni <jni@apache.org>
Committed: Tue Feb 21 00:29:56 2017 -0800

----------------------------------------------------------------------
 .../exec/physical/impl/join/JoinUtils.java      | 20 +++++++++++------
 .../exec/planner/common/DrillJoinRelBase.java   | 15 ++-----------
 .../planner/physical/NestedLoopJoinPrule.java   |  6 ++---
 .../physical/impl/join/TestNestedLoopJoin.java  | 23 +++++++++++++++++---
 4 files changed, 38 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/3ba24fb0/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinUtils.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinUtils.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinUtils.java
index caa18be..1f84965 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinUtils.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinUtils.java
@@ -1,4 +1,4 @@
-/**
+/*
  * 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
@@ -28,25 +28,19 @@ import org.apache.calcite.plan.RelOptUtil;
 import org.apache.calcite.plan.volcano.RelSubset;
 import org.apache.drill.exec.physical.impl.common.Comparator;
 import org.apache.drill.exec.planner.logical.DrillAggregateRel;
-import org.apache.drill.exec.planner.logical.DrillFilterRel;
-import org.apache.drill.exec.planner.logical.DrillProjectRel;
 import org.apache.drill.common.exceptions.DrillRuntimeException;
 import org.apache.drill.common.expression.ErrorCollector;
 import org.apache.drill.common.expression.ErrorCollectorImpl;
 import org.apache.drill.common.expression.LogicalExpression;
-import org.apache.drill.common.logical.data.JoinCondition;
 import org.apache.drill.common.types.TypeProtos;
 import org.apache.drill.exec.expr.ExpressionTreeMaterializer;
 import org.apache.drill.exec.ops.FragmentContext;
-import org.apache.drill.exec.record.RecordBatch;
 import org.apache.drill.exec.record.VectorAccessible;
 import org.apache.drill.exec.resolver.TypeCastRules;
 
 import java.util.LinkedList;
 import java.util.List;
 
-import com.google.common.collect.Lists;
-
 public class JoinUtils {
 
   public enum JoinCategory {
@@ -257,4 +251,16 @@ public class JoinUtils {
     return JoinCategory.EQUALITY;
   }
 
+  /**
+   * Utility method to check if a any of input RelNodes is provably scalar.
+   *
+   * @param left  the RelNode to be inspected.
+   * @param right the RelNode to be inspected.
+   * @return      Return true if any of the given RelNodes is provably scalar.
+   *              Otherwise, return false
+   */
+  public static boolean hasScalarSubqueryInput(RelNode left, RelNode right) {
+    return isScalarSubquery(left) || isScalarSubquery(right);
+  }
+
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/3ba24fb0/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillJoinRelBase.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillJoinRelBase.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillJoinRelBase.java
index 6bd0e9c..7ec92a1 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillJoinRelBase.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillJoinRelBase.java
@@ -1,4 +1,4 @@
-/**
+/*
  * 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
@@ -28,9 +28,7 @@ import org.apache.drill.exec.planner.cost.DrillCostBase;
 import org.apache.drill.exec.physical.impl.join.JoinUtils;
 import org.apache.drill.exec.physical.impl.join.JoinUtils.JoinCategory;
 import org.apache.drill.exec.planner.cost.DrillCostBase.DrillCostFactory;
-import org.apache.drill.exec.planner.cost.DrillRelOptCost;
 import org.apache.drill.exec.planner.physical.PrelUtil;
-import org.apache.calcite.rel.InvalidRelException;
 import org.apache.calcite.rel.core.Join;
 import org.apache.calcite.rel.core.JoinRelType;
 import org.apache.calcite.rel.RelNode;
@@ -68,7 +66,7 @@ public abstract class DrillJoinRelBase extends Join implements DrillRelNode
{
     if (category == JoinCategory.CARTESIAN || category == JoinCategory.INEQUALITY) {
       if (PrelUtil.getPlannerSettings(planner).isNestedLoopJoinEnabled()) {
         if (PrelUtil.getPlannerSettings(planner).isNlJoinForScalarOnly()) {
-          if (hasScalarSubqueryInput()) {
+          if (JoinUtils.hasScalarSubqueryInput(left, right)) {
             return computeLogicalJoinCost(planner);
           } else {
             /*
@@ -204,13 +202,4 @@ public abstract class DrillJoinRelBase extends Join implements DrillRelNode
{
     return costFactory.makeCost(buildRowCount + probeRowCount, cpuCost, 0, 0, memCost);
   }
 
-  private boolean hasScalarSubqueryInput() {
-    if (JoinUtils.isScalarSubquery(this.getLeft())
-        || JoinUtils.isScalarSubquery(this.getRight())) {
-      return true;
-    }
-
-    return false;
-  }
-
 }

http://git-wip-us.apache.org/repos/asf/drill/blob/3ba24fb0/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/NestedLoopJoinPrule.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/NestedLoopJoinPrule.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/NestedLoopJoinPrule.java
index 2cda15a..bfb47d6 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/NestedLoopJoinPrule.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/NestedLoopJoinPrule.java
@@ -1,4 +1,4 @@
-/**
+/*
  * 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
@@ -49,7 +49,7 @@ public class NestedLoopJoinPrule extends JoinPruleBase {
       PlannerSettings settings) {
     JoinRelType type = join.getJoinType();
 
-    if (! (type == JoinRelType.INNER || type == JoinRelType.LEFT)) {
+    if (!(type == JoinRelType.INNER || (type == JoinRelType.LEFT && JoinUtils.hasScalarSubqueryInput(left,
right)))) {
       return false;
     }
 
@@ -63,7 +63,7 @@ public class NestedLoopJoinPrule extends JoinPruleBase {
     }
 
     if (settings.isNlJoinForScalarOnly()) {
-      if (JoinUtils.isScalarSubquery(left) || JoinUtils.isScalarSubquery(right)) {
+      if (JoinUtils.hasScalarSubqueryInput(left, right)) {
         return true;
       } else {
         return false;

http://git-wip-us.apache.org/repos/asf/drill/blob/3ba24fb0/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestNestedLoopJoin.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestNestedLoopJoin.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestNestedLoopJoin.java
index fa28001..6210022 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestNestedLoopJoin.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestNestedLoopJoin.java
@@ -1,4 +1,4 @@
-/**
+/*
  * 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
@@ -19,12 +19,14 @@
 package org.apache.drill.exec.physical.impl.join;
 
 import org.apache.drill.PlanTestBase;
-import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.common.exceptions.UserRemoteException;
 import org.apache.drill.common.util.TestTools;
-import org.apache.drill.exec.work.foreman.UnsupportedRelOperatorException;
 import org.junit.Ignore;
 import org.junit.Test;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.StringStartsWith.startsWith;
+
 public class TestNestedLoopJoin extends PlanTestBase {
 
   private static String nlpattern = "NestedLoopJoin";
@@ -253,4 +255,19 @@ public class TestNestedLoopJoin extends PlanTestBase {
     test(ENABLE_HJ);
     test(ENABLE_MJ);
   }
+
+  @Test(expected = UserRemoteException.class)
+  public void testExceptionLeftNlJoin() throws Exception {
+    try {
+      test(DISABLE_NLJ_SCALAR);
+      test("select r.r_regionkey, n.n_nationkey from cp.`tpch/nation.parquet` n " +
+            " left join cp.`tpch/region.parquet` r on n.n_regionkey < r.r_regionkey where
n.n_nationkey < 3");
+    } catch (UserRemoteException e) {
+      assertThat("No expected current \"UNSUPPORTED_OPERATION ERROR\"",
+        e.getMessage(), startsWith("UNSUPPORTED_OPERATION ERROR"));
+      throw e;
+    } finally {
+      test("alter session reset `planner.enable_nljoin_for_scalar_only`");
+    }
+  }
 }


Mime
View raw message