diff --git a/core/src/main/codegen/templates/Parser.jj b/core/src/main/codegen/templates/Parser.jj index be994d260bb6..49ef4f6f1a3e 100644 --- a/core/src/main/codegen/templates/Parser.jj +++ b/core/src/main/codegen/templates/Parser.jj @@ -2037,7 +2037,7 @@ SqlNode FromClause() : SqlLiteral joinType; } { - e = Join() + e = TableRef1(ExprContext.ACCEPT_QUERY_OR_JOIN) ( // Comma joins should only occur at top-level in the FROM clause. // Valid: @@ -2046,33 +2046,30 @@ SqlNode FromClause() : // Not valid: // * FROM a CROSS JOIN (b, c) LOOKAHEAD(1) - { joinType = JoinType.COMMA.symbol(getPos()); } - e2 = Join() { - e = new SqlJoin(joinType.getParserPosition(), - e, - SqlLiteral.createBoolean(false, joinType.getParserPosition()), - joinType, - e2, - JoinConditionType.NONE.symbol(SqlParserPos.ZERO), - null); - } + e = JoinOrCommaTable(e) )* { return e; } } -SqlNode Join() : +SqlNode JoinOrCommaTable(SqlNode e) : { - SqlNode e; + SqlNode e2; + SqlLiteral joinType; } { - e = TableRef1(ExprContext.ACCEPT_QUERY_OR_JOIN) - ( - LOOKAHEAD(2) - e = JoinTable(e) - )* - { - return e; + LOOKAHEAD(2) + { joinType = JoinType.COMMA.symbol(getPos()); } + e2 = TableRef1(ExprContext.ACCEPT_QUERY_OR_JOIN) { + return new SqlJoin(joinType.getParserPosition(), + e, + SqlLiteral.createBoolean(false, joinType.getParserPosition()), + joinType, + e2, + JoinConditionType.NONE.symbol(SqlParserPos.ZERO), + null); } +| + e2 = JoinTable(e) { return e2; } } /** Matches "LEFT JOIN t ON ...", "RIGHT JOIN t USING ...", "JOIN t". */ @@ -2228,6 +2225,8 @@ SqlNode TableRef3(ExprContext exprContext, boolean lateral) : tableRef = addLateral(tableRef, lateral) [ tableRef = MatchRecognize(tableRef) ] | + LOOKAHEAD(2) + [ ] // "LATERAL" is implicit with "UNNEST", so ignore { s = span(); } args = ParenthesizedQueryOrCommaList(ExprContext.ACCEPT_SUB_QUERY) [ diff --git a/core/src/main/java/org/apache/calcite/sql/SqlJoin.java b/core/src/main/java/org/apache/calcite/sql/SqlJoin.java index ab08f2d1c9cd..6541afa385a6 100644 --- a/core/src/main/java/org/apache/calcite/sql/SqlJoin.java +++ b/core/src/main/java/org/apache/calcite/sql/SqlJoin.java @@ -36,7 +36,7 @@ */ public class SqlJoin extends SqlCall { static final SqlJoinOperator COMMA_OPERATOR = - new SqlJoinOperator("COMMA-JOIN", 16); + new SqlJoinOperator("COMMA-JOIN", 18); public static final SqlJoinOperator OPERATOR = new SqlJoinOperator("JOIN", 18); 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 2ff73fb2edc2..fffb7591d0be 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java @@ -8238,6 +8238,38 @@ void testGroupExpressionEquivalenceParams() { .fails("Table 'DEPT' not found"); } + /** Test case for + * [CALCITE-6266] + * SqlValidatorException with LATERAL TABLE and JOIN. */ + @Test void testCollectionTableWithLateral3() { + // The cause of [CALCITE-6266] was that CROSS JOIN had higher precedence + // than ",", and therefore "table(ramp(deptno)" was being associated with + // "values". + sql("select *\n" + + "from dept,\n" + + " lateral table(ramp(deptno))\n" + + " cross join (values ('A'), ('B'))") + .ok(); + // As above, using NATURAL JOIN + sql("select *\n" + + "from dept,\n" + + " lateral table(ramp(deptno))\n" + + " natural join emp") + .ok(); + // As above, using comma + sql("select *\n" + + "from emp,\n" + + " lateral (select * from dept where dept.deptno = emp.deptno),\n" + + " emp as e2") + .ok(); + // Without 'as e2', relation name is not unique + sql("select *\n" + + "from emp,\n" + + " lateral (select * from dept where dept.deptno = emp.deptno),\n" + + " ^emp^") + .fails("Duplicate relation name 'EMP' in FROM clause"); + } + @Test void testCollectionTableWithCursorParam() { sql("select * from table(dedup(cursor(select * from emp),'ename'))") .type("RecordType(VARCHAR(1024) NOT NULL NAME) NOT NULL"); diff --git a/core/src/test/resources/sql/lateral.iq b/core/src/test/resources/sql/lateral.iq index b38b0d7a7fdb..b4b9d3b95299 100644 --- a/core/src/test/resources/sql/lateral.iq +++ b/core/src/test/resources/sql/lateral.iq @@ -136,4 +136,81 @@ from "scott".dept, !ok +# LATERAL plus CTEs (WITH) +with dept (deptno, dname) + as (values (10, 'ACCOUNTING'), (20, 'RESEARCH')), + emp (empno, deptno, ename) + as (values (7369, 20, 'SMITH'), (7782, 10, 'CLARK'), (7499, 30, 'ALLEN')) +select * +from dept, + lateral (select * from emp where emp.deptno = dept.deptno) as emp; ++--------+------------+-------+---------+-------+ +| DEPTNO | DNAME | EMPNO | DEPTNO0 | ENAME | ++--------+------------+-------+---------+-------+ +| 10 | ACCOUNTING | 7782 | 10 | CLARK | +| 20 | RESEARCH | 7369 | 20 | SMITH | ++--------+------------+-------+---------+-------+ +(2 rows) + +!ok + +# [CALCITE-6266] SqlValidatorException with LATERAL TABLE and JOIN +# The problem also occurred in ', LATERAL (SELECT ...) JOIN', because +# ',' should had lower precedence than 'JOIN', but should have had equal +# precedence. +with dept (deptno, dname) + as (values (10, 'ACCOUNTING'), (20,'RESEARCH')), + emp (empno, deptno, ename) + as (values (7369, 20, 'SMITH'), (7782, 10, 'CLARK'), (7499, 30, 'ALLEN')) +select * +from dept, + lateral (select * from emp where emp.deptno = dept.deptno) as emp + cross join (values 'A', 'B') as v (v); ++--------+------------+-------+---------+-------+---+ +| DEPTNO | DNAME | EMPNO | DEPTNO0 | ENAME | V | ++--------+------------+-------+---------+-------+---+ +| 10 | ACCOUNTING | 7782 | 10 | CLARK | A | +| 10 | ACCOUNTING | 7782 | 10 | CLARK | B | +| 20 | RESEARCH | 7369 | 20 | SMITH | A | +| 20 | RESEARCH | 7369 | 20 | SMITH | B | ++--------+------------+-------+---------+-------+---+ +(4 rows) + +!ok + +# UNNEST applied to VALUES +with t (x, ys) + as (values (1, array [2,3]), (4, array [5])) +select * +from t, + unnest(t.ys) as y; ++---+--------+---+ +| X | YS | Y | ++---+--------+---+ +| 1 | [2, 3] | 2 | +| 1 | [2, 3] | 3 | +| 4 | [5] | 5 | ++---+--------+---+ +(3 rows) + +!ok + +# LATERAL UNNEST means the same as UNNEST +# (Have checked Postgres) +with t (x, ys) + as (values (1, array [2,3]), (4, array [5])) +select * +from t, + lateral unnest(t.ys) as y; ++---+--------+---+ +| X | YS | Y | ++---+--------+---+ +| 1 | [2, 3] | 2 | +| 1 | [2, 3] | 3 | +| 4 | [5] | 5 | ++---+--------+---+ +(3 rows) + +!ok + # End lateral.iq diff --git a/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java b/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java index 5212736ebf4f..31b628b3b883 100644 --- a/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java +++ b/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java @@ -20,6 +20,7 @@ import org.apache.calcite.sql.SqlDialect; import org.apache.calcite.sql.SqlExplain; import org.apache.calcite.sql.SqlIdentifier; +import org.apache.calcite.sql.SqlJoin; import org.apache.calcite.sql.SqlKind; import org.apache.calcite.sql.SqlLambda; import org.apache.calcite.sql.SqlLiteral; @@ -75,6 +76,7 @@ import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.notNullValue; @@ -86,6 +88,8 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeFalse; +import static java.util.Objects.requireNonNull; + /** * A SqlParserTest is a unit-test for * {@link SqlParser the SQL parser}. @@ -676,6 +680,19 @@ private static Matcher isQuoted(final int i, }; } + /** Returns a {@link Matcher} that calls a consumer and then succeeds. + * The consumer should contain custom code, and should fail if it doesn't + * like what it sees. */ + public static Matcher customMatches(String description, + Consumer consumer) { + return new CustomTypeSafeMatcher(description) { + @Override protected boolean matchesSafely(SqlNode sqlNode) { + consumer.accept(sqlNode); + return true; + } + }; + } + protected SortedSet getReservedKeywords() { return keywords("c"); } @@ -3203,6 +3220,28 @@ void checkPeriodPredicate(Checker checker) { + "CROSS JOIN `B`"); } + @Test void testJoinCrossComma() { + sql("select * from a as a2, b cross join c") + .node( + customMatches("custom", node -> { + // Parsed as left-deep: + // select * from (a as a2, b) cross join c + // (This is not valid SQL, but illustrates operator + // associativity.) + assertThat(node, instanceOf(SqlSelect.class)); + final SqlSelect select = (SqlSelect) node; + assertThat(select.getFrom(), instanceOf(SqlJoin.class)); + final SqlJoin from = requireNonNull((SqlJoin) select.getFrom()); + assertThat(from.getLeft(), instanceOf(SqlJoin.class)); + assertThat(from.getRight(), instanceOf(SqlIdentifier.class)); + })); + } + + @Test void testInternalComma() { + sql("select * from (a^,^ b) cross join c") + .fails("(?s)Encountered \",\" at .*"); + } + @Test void testJoinOn() { sql("select * from a left join b on 1 = 1 and 2 = 2 where 3 = 3") .ok("SELECT *\n" @@ -4362,7 +4401,6 @@ void checkPeriodPredicate(Checker checker) { + "Was expecting one of:\n" + " \"LATERAL\" \\.\\.\\.\n" + " \"TABLE\" \\.\\.\\.\n" - + " \"UNNEST\" \\.\\.\\.\n" + " \\.\\.\\.\n" + " \\.\\.\\.\n" + " \\.\\.\\.\n" @@ -4370,7 +4408,8 @@ void checkPeriodPredicate(Checker checker) { + " \\.\\.\\.\n" + " \\.\\.\\.\n" + " \\.\\.\\.\n" - + " \"\\(\" \\.\\.\\.\n.*"); + + " \"\\(\" \\.\\.\\.\n.*" + + " \"UNNEST\" \\.\\.\\.\n.*"); } @Test void testEmptyValues() { @@ -6061,8 +6100,8 @@ private static Matcher isCharLiteral(String s) { + "FROM (VALUES (ROW(1))) AS `X`)))"); sql("SELECT multiset(SELECT x FROM (VALUES(1)) x ^ORDER^ BY x)") .fails("(?s)Encountered \"ORDER\" at.*"); - sql("SELECT multiset(SELECT x FROM (VALUES(1)) x, ^SELECT^ x FROM (VALUES(1)) x)") - .fails("(?s)Incorrect syntax near the keyword 'SELECT' at.*"); + sql("SELECT multiset(SELECT x FROM (VALUES(1)) x^,^ SELECT x FROM (VALUES(1)) x)") + .fails("(?s)Encountered \", SELECT\" at.*"); sql("SELECT multiset(^1^, SELECT x FROM (VALUES(1)) x)") .fails("(?s)Non-query expression encountered in illegal context"); } @@ -6169,8 +6208,8 @@ private static Matcher isCharLiteral(String s) { .ok("SELECT (ARRAY (SELECT `X`\n" + "FROM (VALUES (ROW(1))) AS `X`\n" + "ORDER BY `X`))"); - sql("SELECT array(SELECT x FROM (VALUES(1)) x, ^SELECT^ x FROM (VALUES(1)) x)") - .fails("(?s)Incorrect syntax near the keyword 'SELECT' at.*"); + sql("SELECT array(SELECT x FROM (VALUES(1)) x^,^ SELECT x FROM (VALUES(1)) x)") + .fails("(?s)Encountered \", SELECT\" at.*"); sql("SELECT array(1, ^SELECT^ x FROM (VALUES(1)) x)") .fails("(?s)Incorrect syntax near the keyword 'SELECT'.*"); } @@ -6309,8 +6348,8 @@ private static Matcher isCharLiteral(String s) { sql("SELECT map(1, ^SELECT^ x FROM (VALUES(1)) x)") .fails("(?s)Incorrect syntax near the keyword 'SELECT'.*"); - sql("SELECT map(SELECT x FROM (VALUES(1)) x, ^SELECT^ x FROM (VALUES(1)) x)") - .fails("(?s)Incorrect syntax near the keyword 'SELECT' at.*"); + sql("SELECT map(SELECT x FROM (VALUES(1)) x^,^ SELECT x FROM (VALUES(1)) x)") + .fails("(?s)Encountered \", SELECT\" at.*"); } @Test void testVisitSqlInsertWithSqlShuttle() { @@ -7181,9 +7220,16 @@ private static Consumer> checkWarnings( + "UNNEST(`DEPT`.`EMPLOYEES`, `DEPT`.`MANAGERS`)"; sql(sql).ok(expected); - // LATERAL UNNEST is not valid - sql("select * from dept, lateral ^unnest^(dept.employees)") - .fails("(?s)Encountered \"unnest\" at .*"); + // LATERAL UNNEST is the same as UNNEST + // (LATERAL is implicit for UNNEST, so the parser just ignores it) + sql("select * from dept, lateral unnest(dept.employees)") + .ok("SELECT *\n" + + "FROM `DEPT`,\n" + + "UNNEST(`DEPT`.`EMPLOYEES`)"); + sql("select * from dept, unnest(dept.employees)") + .ok("SELECT *\n" + + "FROM `DEPT`,\n" + + "UNNEST(`DEPT`.`EMPLOYEES`)"); // Does not generate extra parentheses around UNNEST because UNNEST is // a table expression.