You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by vi...@apache.org on 2018/11/15 23:51:30 UTC
[drill] 01/07: DRILL-786: Allow CROSS JOIN syntax
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 <ih...@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);
+ }
+}