asfgit closed pull request #1488: DRILL-786: Allow CROSS JOIN syntax
URL: https://github.com/apache/drill/pull/1488
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/physical/impl/join/JoinUtils.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/JoinUtils.java
index 52871e2b82b..90e85581cd3 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.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 @@
}
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.
@@ -127,6 +135,18 @@ public static boolean checkCartesianJoin(RelNode relNode,
List<Integer> leftKeys
return false;
}
+ /**
+ * 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.
@@ -299,6 +319,16 @@ public static boolean hasScalarSubqueryInput(RelNode left,
RelNode right) {
return isScalarSubquery(left) || isScalarSubquery(right);
}
+ /**
+ * 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 c75311f4ff1..f7d11f8a9bc 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.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 @@ protected DrillRel convertToRawDrel(final RelNode relNode)
throws SqlUnsupported
} 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 @@ protected Prel convertToPrel(RelNode drel, RelDataType
validatedRowType) throws
} 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 @@ protected Prel convertToPrel(RelNode drel, RelDataType
validatedRowType) throws
} 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 8505a68a108..b9728410a97 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.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 SqlNode visit(SqlCall sqlCall) {
"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 679d01eeb05..1dcd6919956 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 void testDisabledNaturalJoin() throws Exception {
}
}
- @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 00000000000..348df328c92
--- /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);
+ }
+}
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
