drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From vita...@apache.org
Subject [drill] 01/07: DRILL-786: Allow CROSS JOIN syntax
Date Thu, 15 Nov 2018 23:51:30 GMT
This is an automated email from the ASF dual-hosted git repository.

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

commit cab059abc0c0f6044e9aa15a1747e8542ac5ad46
Author: Igor Guzenko <ihor.huzenko.igs@gmail.com>
AuthorDate: Wed Oct 3 14:55:31 2018 +0300

    DRILL-786: Allow CROSS JOIN syntax
    
     1. Removed throw statement in UnsupportedOperatorsVisitor
     2. Extended UnsupportedRelOperatorException's message
    
    closes #1488
---
 .../drill/exec/physical/impl/join/JoinUtils.java   |  30 +++
 .../planner/sql/handlers/DefaultSqlHandler.java    |  14 +-
 .../sql/parser/UnsupportedOperatorsVisitor.java    |   9 -
 .../apache/drill/TestDisabledFunctionality.java    |  10 +-
 .../drill/exec/planner/sql/CrossJoinTest.java      | 201 +++++++++++++++++++++
 5 files changed, 238 insertions(+), 26 deletions(-)

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 52871e2..90e8558 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
@@ -51,11 +51,14 @@ import org.apache.drill.exec.ops.FragmentContext;
 import org.apache.drill.exec.planner.logical.DrillLimitRel;
 import org.apache.drill.exec.record.VectorAccessible;
 import org.apache.drill.exec.resolver.TypeCastRules;
+import org.apache.drill.exec.work.foreman.UnsupportedRelOperatorException;
 
 import java.util.ArrayList;
 import java.util.LinkedList;
 import java.util.List;
 
+import static org.apache.drill.exec.planner.physical.PlannerSettings.NLJOIN_FOR_SCALAR;
+
 public class JoinUtils {
 
   public enum JoinCategory {
@@ -65,6 +68,11 @@ public class JoinUtils {
   }
   private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(JoinUtils.class);
 
+  public static final String FAILED_TO_PLAN_CARTESIAN_JOIN = String.format(
+      "This query cannot be planned possibly due to either a cartesian join or an inequality
join. %n" +
+          "If a cartesian or inequality join is used intentionally, set the option '%s' to
false and try again.",
+      NLJOIN_FOR_SCALAR.getOptionName());
+
   // Check the comparator is supported in join condition. Note that a similar check is also
   // done in JoinPrel; however we have to repeat it here because a physical plan
   // may be submitted directly to Drill.
@@ -128,6 +136,18 @@ public class JoinUtils {
   }
 
   /**
+   * Check if the given RelNode contains any Cartesian join.
+   * Return true if find one. Otherwise, return false.
+   *
+   * @param relNode     {@link RelNode} instance to be inspected
+   * @return            Return true if the given relNode contains Cartesian join.
+   *                    Otherwise, return false
+   */
+  public static boolean checkCartesianJoin(RelNode relNode) {
+    return checkCartesianJoin(relNode, new LinkedList<>(), new LinkedList<>(),
new LinkedList<>());
+  }
+
+  /**
    * Checks if implicit cast is allowed between the two input types of the join condition.
Currently we allow
    * implicit casts in join condition only between numeric types and varchar/varbinary types.
    * @param input1
@@ -300,6 +320,16 @@ public class JoinUtils {
   }
 
   /**
+   * Creates new exception for queries that cannot be planned due
+   * to presence of cartesian or inequality join.
+   *
+   * @return new {@link UnsupportedRelOperatorException} instance
+   */
+  public static UnsupportedRelOperatorException cartesianJoinPlanningException() {
+    return new UnsupportedRelOperatorException(FAILED_TO_PLAN_CARTESIAN_JOIN);
+  }
+
+  /**
    * Collects expressions list from the input project.
    * For the case when input rel node has single input, its input is taken.
    */
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
index c75311f..f7d11f8 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java
@@ -18,7 +18,6 @@
 package org.apache.drill.exec.planner.sql.handlers;
 
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
@@ -106,7 +105,6 @@ import org.apache.drill.exec.server.options.OptionManager;
 import org.apache.drill.exec.util.Pointer;
 import org.apache.drill.exec.work.foreman.ForemanSetupException;
 import org.apache.drill.exec.work.foreman.SqlUnsupportedException;
-import org.apache.drill.exec.work.foreman.UnsupportedRelOperatorException;
 import org.slf4j.Logger;
 
 import org.apache.drill.shaded.guava.com.google.common.base.Preconditions;
@@ -294,8 +292,8 @@ public class DefaultSqlHandler extends AbstractSqlHandler {
     } catch (RelOptPlanner.CannotPlanException ex) {
       logger.error(ex.getMessage());
 
-      if (JoinUtils.checkCartesianJoin(relNode, new ArrayList<>(), new ArrayList<>(),
new ArrayList<>())) {
-        throw new UnsupportedRelOperatorException("This query cannot be planned possibly
due to either a cartesian join or an inequality join");
+      if (JoinUtils.checkCartesianJoin(relNode)) {
+        throw JoinUtils.cartesianJoinPlanningException();
       } else {
         throw ex;
       }
@@ -459,8 +457,8 @@ public class DefaultSqlHandler extends AbstractSqlHandler {
     } catch (RelOptPlanner.CannotPlanException ex) {
       logger.error(ex.getMessage());
 
-      if (JoinUtils.checkCartesianJoin(drel, new ArrayList<>(), new ArrayList<>(),
new ArrayList<>())) {
-        throw new UnsupportedRelOperatorException("This query cannot be planned possibly
due to either a cartesian join or an inequality join");
+      if (JoinUtils.checkCartesianJoin(drel)) {
+        throw JoinUtils.cartesianJoinPlanningException();
       } else {
         throw ex;
       }
@@ -482,8 +480,8 @@ public class DefaultSqlHandler extends AbstractSqlHandler {
       } catch (RelOptPlanner.CannotPlanException ex) {
         logger.error(ex.getMessage());
 
-        if (JoinUtils.checkCartesianJoin(drel, new ArrayList<>(), new ArrayList<>(),
new ArrayList<>())) {
-          throw new UnsupportedRelOperatorException("This query cannot be planned possibly
due to either a cartesian join or an inequality join");
+        if (JoinUtils.checkCartesianJoin(drel)) {
+          throw JoinUtils.cartesianJoinPlanningException();
         } else {
           throw ex;
         }
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java
index 8505a68..b972841 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/parser/UnsupportedOperatorsVisitor.java
@@ -36,7 +36,6 @@ import org.apache.calcite.sql.fun.SqlCountAggFunction;
 import org.apache.calcite.sql.SqlCall;
 import org.apache.calcite.sql.SqlKind;
 import org.apache.calcite.sql.SqlJoin;
-import org.apache.calcite.sql.JoinType;
 import org.apache.calcite.sql.SqlNode;
 import org.apache.calcite.sql.type.SqlTypeName;
 import org.apache.calcite.sql.util.SqlShuttle;
@@ -256,14 +255,6 @@ public class UnsupportedOperatorsVisitor extends SqlShuttle {
             "See Apache Drill JIRA: DRILL-1986");
         throw new UnsupportedOperationException();
       }
-
-      // Block Cross Join
-      if(join.getJoinType() == JoinType.CROSS) {
-        unsupportedOperatorCollector.setException(SqlUnsupportedException.ExceptionType.RELATIONAL,
-            "CROSS JOIN is not supported\n" +
-            "See Apache Drill JIRA: DRILL-1921");
-        throw new UnsupportedOperationException();
-      }
     }
 
     //Disable UNNEST if the configuration disable it
diff --git a/exec/java-exec/src/test/java/org/apache/drill/TestDisabledFunctionality.java
b/exec/java-exec/src/test/java/org/apache/drill/TestDisabledFunctionality.java
index 679d01e..1dcd691 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/TestDisabledFunctionality.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/TestDisabledFunctionality.java
@@ -16,6 +16,7 @@
  * limitations under the License.
  */
 package org.apache.drill;
+
 import org.apache.drill.categories.UnlikelyTest;
 import org.apache.drill.common.exceptions.UserException;
 import org.apache.drill.exec.planner.physical.PlannerSettings;
@@ -92,15 +93,6 @@ public class TestDisabledFunctionality extends BaseTestQuery {
     }
   }
 
-  @Test(expected = UnsupportedRelOperatorException.class) // see DRILL-1921
-  public void testDisabledCrossJoin() throws Exception {
-    try {
-      test("select * from cp.`tpch/nation.parquet` CROSS JOIN cp.`tpch/region.parquet`");
-    } catch(UserException ex) {
-      throwAsUnsupportedException(ex);
-    }
-  }
-
   @Test(expected = UnsupportedDataTypeException.class) // see DRILL-1959
   public void testDisabledCastTINYINT() throws Exception {
     try {
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/planner/sql/CrossJoinTest.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/planner/sql/CrossJoinTest.java
new file mode 100644
index 0000000..348df32
--- /dev/null
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/planner/sql/CrossJoinTest.java
@@ -0,0 +1,201 @@
+/*
+ * 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.exec.planner.sql;
+
+import org.apache.drill.categories.SqlTest;
+import org.apache.drill.common.exceptions.UserRemoteException;
+import org.apache.drill.exec.planner.physical.PlannerSettings;
+import org.apache.drill.test.ClusterFixture;
+import org.apache.drill.test.ClusterTest;
+import org.junit.After;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import static org.apache.drill.exec.physical.impl.join.JoinUtils.FAILED_TO_PLAN_CARTESIAN_JOIN;
+
+@Category(SqlTest.class)
+public class CrossJoinTest extends ClusterTest {
+
+  private static int NATION_TABLE_RECORDS_COUNT = 25;
+
+  private static int EXPECTED_COUNT = NATION_TABLE_RECORDS_COUNT * NATION_TABLE_RECORDS_COUNT;
+
+  @BeforeClass
+  public static void setUp() throws Exception {
+    startCluster(ClusterFixture.builder(dirTestWatcher));
+  }
+
+  @After
+  public void tearDown() {
+    client.resetSession(PlannerSettings.NLJOIN_FOR_SCALAR.getOptionName());
+  }
+
+  @Test
+  public void testCrossJoinFailsForEnabledOption() throws Exception {
+    enableNlJoinForScalarOnly();
+
+    thrownException.expect(UserRemoteException.class);
+    thrownException.expectMessage(FAILED_TO_PLAN_CARTESIAN_JOIN);
+
+    queryBuilder().sql(
+        "SELECT l.n_name, r.n_name " +
+            "FROM cp.`tpch/nation.parquet` l " +
+            "CROSS JOIN cp.`tpch/nation.parquet` r")
+        .run();
+  }
+
+  @Test
+  public void testCrossJoinSucceedsForDisabledOption() throws Exception {
+    disableNlJoinForScalarOnly();
+    client.testBuilder().sqlQuery(
+        "SELECT l.n_name,r.n_name " +
+            "FROM cp.`tpch/nation.parquet` l " +
+            "CROSS JOIN cp.`tpch/nation.parquet` r")
+        .expectsNumRecords(EXPECTED_COUNT)
+        .go();
+  }
+
+  @Test
+  public void testCommaJoinFailsForEnabledOption() throws Exception {
+    enableNlJoinForScalarOnly();
+
+    thrownException.expect(UserRemoteException.class);
+    thrownException.expectMessage(FAILED_TO_PLAN_CARTESIAN_JOIN);
+
+    queryBuilder().sql(
+        "SELECT l.n_name,r.n_name " +
+            "FROM cp.`tpch/nation.parquet` l, cp.`tpch/nation.parquet` r")
+        .run();
+  }
+
+  @Test
+  public void testCommaJoinSucceedsForDisabledOption() throws Exception {
+    disableNlJoinForScalarOnly();
+    client.testBuilder().sqlQuery(
+        "SELECT l.n_name,r.n_name " +
+            "FROM cp.`tpch/nation.parquet` l, cp.`tpch/nation.parquet` r")
+        .expectsNumRecords(EXPECTED_COUNT)
+        .go();
+  }
+
+  @Test
+  public void testSubSelectCrossJoinFailsForEnabledOption() throws Exception {
+    enableNlJoinForScalarOnly();
+
+    thrownException.expect(UserRemoteException.class);
+    thrownException.expectMessage(FAILED_TO_PLAN_CARTESIAN_JOIN);
+
+    queryBuilder().sql(
+        "SELECT COUNT(*) c " +
+            "FROM (" +
+            "SELECT l.n_name,r.n_name " +
+            "FROM cp.`tpch/nation.parquet` l " +
+            "CROSS JOIN cp.`tpch/nation.parquet` r" +
+            ")")
+        .run();
+  }
+
+  @Test
+  public void testSubSelectCrossJoinSucceedsForDisabledOption() throws Exception {
+    disableNlJoinForScalarOnly();
+
+    client.testBuilder()
+        .sqlQuery(
+            "SELECT COUNT(*) c " +
+                "FROM (SELECT l.n_name,r.n_name " +
+                "FROM cp.`tpch/nation.parquet` l " +
+                "CROSS JOIN cp.`tpch/nation.parquet` r)")
+        .unOrdered()
+        .baselineColumns("c")
+        .baselineValues((long) EXPECTED_COUNT)
+        .go();
+  }
+
+  @Test
+  public void textCrossAndCommaJoinFailsForEnabledOption() throws Exception {
+    enableNlJoinForScalarOnly();
+
+    thrownException.expect(UserRemoteException.class);
+    thrownException.expectMessage(FAILED_TO_PLAN_CARTESIAN_JOIN);
+
+    queryBuilder().sql(
+        "SELECT * " +
+            "FROM cp.`tpch/nation.parquet` a, cp.`tpch/nation.parquet` b " +
+            "CROSS JOIN cp.`tpch/nation.parquet` c")
+        .run();
+  }
+
+  @Test
+  public void textCrossAndCommaJoinSucceedsForDisabledOption() throws Exception {
+    disableNlJoinForScalarOnly();
+
+    client.testBuilder().sqlQuery(
+        "SELECT * " +
+            "FROM cp.`tpch/nation.parquet` a, cp.`tpch/nation.parquet` b " +
+            "CROSS JOIN cp.`tpch/nation.parquet` c")
+        .expectsNumRecords(NATION_TABLE_RECORDS_COUNT * EXPECTED_COUNT)
+        .go();
+  }
+
+  @Test
+  public void testCrossApplyFailsForEnabledOption() throws Exception {
+    enableNlJoinForScalarOnly();
+
+    thrownException.expect(UserRemoteException.class);
+    thrownException.expectMessage(FAILED_TO_PLAN_CARTESIAN_JOIN);
+
+    queryBuilder().sql(
+        "SELECT * " +
+            "FROM cp.`tpch/nation.parquet` l " +
+            "CROSS APPLY cp.`tpch/nation.parquet` r")
+        .run();
+  }
+
+  @Test
+  public void testCrossApplySucceedsForDisabledOption() throws Exception {
+    disableNlJoinForScalarOnly();
+
+    client.testBuilder().sqlQuery(
+        "SELECT * " +
+            "FROM cp.`tpch/nation.parquet` l " +
+            "CROSS APPLY cp.`tpch/nation.parquet` r")
+        .expectsNumRecords(EXPECTED_COUNT)
+        .go();
+  }
+
+  @Test
+  public void testCrossJoinSucceedsForEnabledOptionAndScalarInput() throws Exception {
+    enableNlJoinForScalarOnly();
+
+    client.testBuilder().sqlQuery(
+        "SELECT * " +
+            "FROM cp.`tpch/nation.parquet` l " +
+            "CROSS JOIN (SELECT * FROM cp.`tpch/nation.parquet` r LIMIT 1)")
+        .expectsNumRecords(NATION_TABLE_RECORDS_COUNT)
+        .go();
+  }
+
+  private static void disableNlJoinForScalarOnly() {
+    client.alterSession(PlannerSettings.NLJOIN_FOR_SCALAR.getOptionName(), false);
+  }
+
+  private static void enableNlJoinForScalarOnly() {
+    client.alterSession(PlannerSettings.NLJOIN_FOR_SCALAR.getOptionName(), true);
+  }
+}


Mime
View raw message