Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Coral-Trino] Support CROSS JOIN for Correlated Inner Queries #493

Merged
merged 2 commits into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2023 LinkedIn Corporation. All rights reserved.
* Copyright 2023-2024 LinkedIn Corporation. All rights reserved.
* Licensed under the BSD-2 Clause license.
* See LICENSE in the project root for license information.
*/
Expand All @@ -8,6 +8,7 @@
import org.apache.calcite.sql.JoinConditionType;
import org.apache.calcite.sql.JoinType;
import org.apache.calcite.sql.SqlCall;
import org.apache.calcite.sql.SqlIdentifier;
import org.apache.calcite.sql.SqlJoin;
import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.SqlLiteral;
Expand Down Expand Up @@ -40,6 +41,8 @@
* left: `default`.`complex` joinType: CROSS JOIN right: UNNEST(`complex`.`c`) AS `t_alias` (`col_alias`)
*/
public class JoinSqlCallTransformer extends SqlCallTransformer {
private static final String TRANSFORM_OPERATOR = "transform";

@Override
protected boolean condition(SqlCall sqlCall) {
return sqlCall.getOperator().kind == SqlKind.JOIN && ((SqlJoin) sqlCall).getJoinType() == JoinType.COMMA;
Expand All @@ -51,11 +54,14 @@ protected SqlCall transform(SqlCall sqlCall) {

// Check if there's an unnest SqlCall present in the nested SqlNodes
if (isUnnestOperatorPresentInRightSqlNode(joinSqlCall.getRight())) {
// Check if the unnest SqlCall is uncorrelated with the SqlJoin SqlCall
if (isUnnestSqlCallCorrelated(joinSqlCall.getRight())) {
// Check if the nested UNNEST SqlCall is correlated to the outer SQL query
SqlCall unnestCall = ((SqlCall) joinSqlCall.getRight()).operand(0);
if (isSqlCallCorrelated(unnestCall)) {
// Substitute COMMA JOIN with CROSS JOIN
return createCrossJoinSqlCall(joinSqlCall);
} else {
// If the unnest SqlCall is uncorrelated to the outer SQL query, for example,
// when the unnest operand is an inline defined array, do not substitute JoinType
return joinSqlCall;
}
} else {
Expand All @@ -75,24 +81,37 @@ private static boolean isUnnestOperatorPresentInRightSqlNode(SqlNode rightSqlNod
&& ((SqlCall) rightSqlNode).operand(0).getKind() == SqlKind.UNNEST;
}

private static boolean isUnnestSqlCallCorrelated(SqlNode sqlNode) {
SqlNode aliasOperand = ((SqlCall) sqlNode).operand(0); // unnest(x)
SqlNode unnestOperand = ((SqlCall) aliasOperand).operand(0); // x

// When the unnest operand, 'x', is:
// (1) SqlIdentifier referring to a column, ex: table1.col1
// (2) SqlCall with "IF" operator for outer unnest
// (3) SqlCall with "TRANSFORM" operator to support unnesting array of structs
// Substitute JoinType with CROSS JoinType.
if (unnestOperand.getKind() == SqlKind.IDENTIFIER
|| (unnestOperand instanceof SqlCall
&& ((SqlCall) unnestOperand).getOperator().getName().equalsIgnoreCase("transform"))
|| (unnestOperand instanceof SqlCall
&& ((SqlCall) unnestOperand).getOperator().getName().equalsIgnoreCase("if"))) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the "if" case here lost because we now recurse using isSqlCallCorrelation into the operands of an "if" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"if" is another SqlOperator. So now we recursively examine the operands of If operator, if and when encountered.

return true;
/**
* Given an Unnest SqlCall, UNNEST('x'), the SqlCall is considered correlated when the unnest operand, 'x':
* (1) References a column from the base tables:
* Sample SqlCalls:
* - UNNEST(table1.col)
* - UNNEST(if(table1.col IS NOT NULL AND CAST(CARDINALITY(table1.col) AS INTEGER) > 0 table1.col, ARRAY[NULL] WITH ORDINALITY))
* - UNNEST(split(table1.stringCol, "delimiter"))
* (2) SqlCall with "TRANSFORM" operator to support unnesting array of structs
* Sample SqlCalls:
* - UNNEST(TRANSFORM(table1.col, x -> ROW(x)))
* @param unnestSqlCall unnest sqlCall
* @return true if the sqlCall is correlated to the outer query
*/
private static boolean isSqlCallCorrelated(SqlCall unnestSqlCall) {
for (SqlNode operand : unnestSqlCall.getOperandList()) {
if (operand instanceof SqlIdentifier) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we know for sure that any identifier operand of an unnest operator references a column from the base tables? Could it be a reference to a non-base table column?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all tables referenced can be seen as base tables.

return true;
} else if (operand instanceof SqlCall) {
/**
* transform sqlCall: TRANSFORM(table1.col, x -> ROW(x))
* operator = "TRANSFORM"
* operand = "table1.col, x -> ROW(x)"
*/
if (((SqlCall) operand).getOperator().getName().equalsIgnoreCase(TRANSFORM_OPERATOR)) {
return true;
}
if (isSqlCallCorrelated((SqlCall) operand)) {
return true;
}
}
}
// If the unnest SqlCall is uncorrelated with the SqlJoin, for example,
// when the unnest operand is an inline defined array, do not substitute JoinType
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,18 @@ public void testLateralViewArray2() {
assertEquals(expandedSql, targetSql);
}

@Test
public void testLateralViewArray3() {
RelNode relNode = TestUtils.getHiveToRelConverter().convertSql(
"SELECT arr.alias FROM test.tableA tmp LATERAL VIEW EXPLODE(split(tmp.b.b1, 'delim')) arr as alias");

String targetSql = "SELECT \"t0\".\"alias\" AS \"alias\"\n" + "FROM \"test\".\"tablea\" AS \"tablea\"\n"
+ "CROSS JOIN UNNEST(\"split\"(\"tablea\".\"b\".\"b1\", 'delim')) AS \"t0\" (\"alias\")";
RelToTrinoConverter relToTrinoConverter = TestUtils.getRelToTrinoConverter();
String expandedSql = relToTrinoConverter.convert(relNode);
assertEquals(expandedSql, targetSql);
}

@Test
public void testLateralViewArrayWithoutColumns() {
RelNode relNode = TestUtils.getHiveToRelConverter()
Expand Down
Loading