Skip to content

Commit

Permalink
DRILL-786: Allow CROSS JOIN syntax
Browse files Browse the repository at this point in the history
 1. Removed throw statement in UnsupportedOperatorsVisitor
 2. Extended UnsupportedRelOperatorException's message

closes #1488
  • Loading branch information
ihuzenko authored and vdiravka committed Nov 15, 2018
1 parent 047c512 commit cab059a
Show file tree
Hide file tree
Showing 5 changed files with 238 additions and 26 deletions.
Expand Up @@ -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 {
Expand All @@ -65,6 +68,11 @@ public enum JoinCategory {
}
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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
@@ -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);
}
}

0 comments on commit cab059a

Please sign in to comment.