From fce66f14f6bea7db1f906d357288f41ffae48500 Mon Sep 17 00:00:00 2001 From: Kevin Ge Date: Tue, 29 Oct 2024 13:30:02 -0400 Subject: [PATCH 1/4] preserve lateral unnest --- .../java/org/apache/calcite/sql/SqlUtil.java | 12 +++++++++++ .../sql/validate/SqlValidatorImpl.java | 21 +++++++------------ .../calcite/sql2rel/SqlToRelConverter.java | 15 ++++++------- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/sql/SqlUtil.java b/core/src/main/java/org/apache/calcite/sql/SqlUtil.java index 8012196e7..8214c4029 100644 --- a/core/src/main/java/org/apache/calcite/sql/SqlUtil.java +++ b/core/src/main/java/org/apache/calcite/sql/SqlUtil.java @@ -31,6 +31,7 @@ import org.apache.calcite.sql.type.SqlTypeName; import org.apache.calcite.sql.type.SqlTypeUtil; import org.apache.calcite.sql.util.SqlBasicVisitor; +import org.apache.calcite.sql.util.SqlShuttle; import org.apache.calcite.sql.validate.SqlNameMatcher; import org.apache.calcite.sql.validate.SqlValidatorUtil; import org.apache.calcite.util.BarfingInvocationHandler; @@ -1079,6 +1080,17 @@ private void visitChild(SqlNode node) { return check(type); } } + + /** Walks over a {@link org.apache.calcite.sql.SqlNode} tree and removes any + * instance of a LATERAL operator. */ + public static class LateralRemover extends SqlShuttle { + @Override public SqlNode visit(SqlCall call) { + if (call.getKind() == SqlKind.LATERAL) { + return call.operand(0).accept(this); + } + return super.visit(call); + } + } } // End SqlUtil.java diff --git a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java index 249bd7e7a..b592b4d68 100644 --- a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java +++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java @@ -2293,8 +2293,12 @@ private SqlNode registerFrom( alias, forceNullable); - // Keep preceding "LATERAL" keyword from joins with inner SELECT subquery - if (lateral && kind == SqlKind.SELECT) { + // Preserve "LATERAL" keyword in validated node: + // 1. Joins with inner SELECT subquery preceded by LATERAL + // Example: SELECT ... FROM LATERAL(SELECT ...) AS t + // 2. Joins with UNNEST operator preceded by LATERAL + // Example: SELECT ... FROM LATERAL(UNNEST ...) AS t + if (lateral && (kind == SqlKind.SELECT) || (kind == SqlKind.UNNEST)) { SqlNode lateralNode = SqlStdOperatorTable.LATERAL.createCall(POS, newNode); return lateralNode; @@ -3120,6 +3124,7 @@ protected void validateFrom( SqlValidatorScope scope) { Objects.requireNonNull(targetRowType); switch (node.getKind()) { + case LATERAL: case AS: validateFrom( ((SqlCall) node).operand(0), @@ -3138,10 +3143,6 @@ protected void validateFrom( case UNNEST: validateUnnest((SqlCall) node, scope, targetRowType); break; - case LATERAL: - // Validate subquery that LATERAL precedes - validateQuery(((SqlCall) node).operand(0), scope, targetRowType); - break; default: validateQuery(node, scope, targetRowType); break; @@ -3149,13 +3150,7 @@ protected void validateFrom( // Validate the namespace representation of the node, just in case the // validation did not occur implicitly. - if (node.getKind() == SqlKind.LATERAL) { - // Skip over fetching for LATERAL namespace since they aren't registered, use subquery instead - getNamespace(((SqlCall) node).operand(0), scope) - .validate(targetRowType); - } else { - getNamespace(node, scope).validate(targetRowType); - } + getNamespace(node, scope).validate(targetRowType); } protected void validateOver(SqlCall call, SqlValidatorScope scope) { diff --git a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java index 453ba2403..0d05be6f9 100644 --- a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java +++ b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java @@ -1993,18 +1993,15 @@ protected void convertFrom( return; } - // Skip over LATERAL conversion - if (from.getKind() == SqlKind.LATERAL) { - from = ((SqlCall) from).operand(0); - } - final SqlCall call; final SqlNode[] operands; switch (from.getKind()) { case MATCH_RECOGNIZE: convertMatchRecognize(bb, (SqlCall) from); return; - + case LATERAL: + convertFrom(bb, ((SqlCall) from).operand(0)); + return; case AS: call = (SqlCall) from; convertFrom(bb, call.operand(0)); @@ -2080,8 +2077,12 @@ protected void convertFrom( ((DelegatingScope) bb.scope).getParent()); final Blackboard leftBlackboard = createBlackboard(leftScope, null, false); + // We don't register the scopes of LATERAL SqlNodes, so we need to + // remove them before looking up the associated scope + // Note that LATERALs could only appear on the right side of a join + SqlNode rightWithoutLaterals = right.accept(new SqlUtil.LateralRemover()); final SqlValidatorScope rightScope = - Util.first(validator.getJoinScope(right), + Util.first(validator.getJoinScope(rightWithoutLaterals), ((DelegatingScope) bb.scope).getParent()); final Blackboard rightBlackboard = createBlackboard(rightScope, null, false); From d027f95bf73ba23cc0124dd420267745aa04b796 Mon Sep 17 00:00:00 2001 From: Kevin Ge Date: Tue, 29 Oct 2024 14:55:46 -0400 Subject: [PATCH 2/4] preserve lateral unnest unit test --- .../apache/calcite/test/SqlValidatorTest.java | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java index d0ed1fcf3..ed11bcbf5 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java @@ -11521,7 +11521,7 @@ private void checkCustomColumnResolving(String table) { * * Preserve LATERAL keyword during validation #98. */ @Test - public void testLateralKeywordExistsAfterValidation() throws SqlParseException { + public void testLateralPreservedInLateralSelect() throws SqlParseException { String sql = "SELECT * FROM emp CROSS JOIN LATERAL " + "(SELECT * FROM dept WHERE deptno = emp.deptno)"; @@ -11531,6 +11531,24 @@ public void testLateralKeywordExistsAfterValidation() throws SqlParseException { assertTrue(validatedNode.toString().contains("LATERAL")); } + + @Test + public void testLateralPreservedInLateralUnnest() throws SqlParseException { + // Per SQL std, UNNEST is implicitly LATERAL + String sql = "select*from unnest(array[1])"; + SqlNode node = tester.parseQuery(sql); + SqlValidator validator = tester.getValidator(); + SqlNode validatedNode = validator.validate(node); + + assertTrue(validatedNode.toString().contains("LATERAL")); + + sql = "select c from unnest(array(select deptno from dept)) as t(c)"; + node = tester.parseQuery(sql); + validator = tester.getValidator(); + validatedNode = validator.validate(node); + + assertTrue(validatedNode.toString().contains("LATERAL")); + } } // End SqlValidatorTest.java From dba8b1d939fcb62083e81b926e0faebf18b89b9e Mon Sep 17 00:00:00 2001 From: Kevin Ge Date: Wed, 30 Oct 2024 15:34:51 -0400 Subject: [PATCH 3/4] modify conditional --- .../java/org/apache/calcite/sql/validate/SqlValidatorImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java index b592b4d68..65aaba8c5 100644 --- a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java +++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java @@ -2298,7 +2298,7 @@ private SqlNode registerFrom( // Example: SELECT ... FROM LATERAL(SELECT ...) AS t // 2. Joins with UNNEST operator preceded by LATERAL // Example: SELECT ... FROM LATERAL(UNNEST ...) AS t - if (lateral && (kind == SqlKind.SELECT) || (kind == SqlKind.UNNEST)) { + if (lateral && (kind == SqlKind.SELECT || kind == SqlKind.UNNEST)) { SqlNode lateralNode = SqlStdOperatorTable.LATERAL.createCall(POS, newNode); return lateralNode; From 66d9531e79ac746ef377a9297637d0f82d572a99 Mon Sep 17 00:00:00 2001 From: Kevin Ge Date: Wed, 30 Oct 2024 19:08:57 -0400 Subject: [PATCH 4/4] only preserve lateral, do not addd --- .../sql/validate/SqlValidatorImpl.java | 26 +++++++++++++++++-- .../apache/calcite/test/SqlValidatorTest.java | 6 ++--- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java index 65aaba8c5..407a237a9 100644 --- a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java +++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java @@ -2006,6 +2006,23 @@ protected void registerNamespace( } } + /** + * @see #registerFrom(SqlValidatorScope, SqlValidatorScope, boolean, SqlNode, SqlNode, String, SqlNodeList, boolean, boolean, boolean) + */ + private SqlNode registerFrom( + SqlValidatorScope parentScope, + SqlValidatorScope usingScope, + boolean register, + final SqlNode node, + SqlNode enclosingNode, + String alias, + SqlNodeList extendList, + boolean forceNullable, + final boolean lateral) { + return registerFrom(parentScope, usingScope, register, node, enclosingNode, alias, + extendList, forceNullable, lateral, false); + } + /** * Registers scopes and namespaces implied a relational expression in the * FROM clause. @@ -2035,6 +2052,9 @@ protected void registerNamespace( * @param lateral Whether LATERAL is specified, so that items to the * left of this in the JOIN tree are visible in the * scope + * @param fromLateral Whether node was nested in a LATERAL node because + * even if LATERAL is specified, there may not be an explicit + * LATERAL node in the tree (e.g. UNNEST is implicitly LATERAL) * @return registered node, usually the same as {@code node} */ private SqlNode registerFrom( @@ -2046,7 +2066,8 @@ private SqlNode registerFrom( String alias, SqlNodeList extendList, boolean forceNullable, - final boolean lateral) { + final boolean lateral, + final boolean fromLateral) { final SqlKind kind = node.getKind(); SqlNode expr; @@ -2248,6 +2269,7 @@ private SqlNode registerFrom( alias, extendList, forceNullable, + true, true); case COLLECTION_TABLE: @@ -2298,7 +2320,7 @@ private SqlNode registerFrom( // Example: SELECT ... FROM LATERAL(SELECT ...) AS t // 2. Joins with UNNEST operator preceded by LATERAL // Example: SELECT ... FROM LATERAL(UNNEST ...) AS t - if (lateral && (kind == SqlKind.SELECT || kind == SqlKind.UNNEST)) { + if (fromLateral && (kind == SqlKind.SELECT || kind == SqlKind.UNNEST)) { SqlNode lateralNode = SqlStdOperatorTable.LATERAL.createCall(POS, newNode); return lateralNode; diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java index ed11bcbf5..3c7d46fac 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java @@ -11533,21 +11533,21 @@ public void testLateralPreservedInLateralSelect() throws SqlParseException { } @Test - public void testLateralPreservedInLateralUnnest() throws SqlParseException { + public void testLateralKeywordIsNotAddedToUnnest() throws SqlParseException { // Per SQL std, UNNEST is implicitly LATERAL String sql = "select*from unnest(array[1])"; SqlNode node = tester.parseQuery(sql); SqlValidator validator = tester.getValidator(); SqlNode validatedNode = validator.validate(node); - assertTrue(validatedNode.toString().contains("LATERAL")); + assertTrue(!validatedNode.toString().contains("LATERAL")); sql = "select c from unnest(array(select deptno from dept)) as t(c)"; node = tester.parseQuery(sql); validator = tester.getValidator(); validatedNode = validator.validate(node); - assertTrue(validatedNode.toString().contains("LATERAL")); + assertTrue(!validatedNode.toString().contains("LATERAL")); } }