This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new 9fa1e16e32 [CALCITE-6266] SqlValidatorException with LATERAL TABLE and
JOIN
9fa1e16e32 is described below
commit 9fa1e16e326e37688b829a2a73064652df40c90d
Author: Julian Hyde <[email protected]>
AuthorDate: Wed Jul 17 13:33:45 2024 -0700
[CALCITE-6266] SqlValidatorException with LATERAL TABLE and JOIN
The current precedence of ',' (the comma operator in
'FROM t1, t2') and 'JOIN' is wrong. JOIN is currently higher,
and so in a query like
SELECT *
FROM dept,
LATERAL (SELECT * FROM emp WHERE emp.deptno = dept.deptno) AS emp
CROSS JOIN (VALUES ('A'), ('B')) AS v (v);
the emp subqery is associated with v more tightly than dept,
and therefore dept.deptno is not visible; the correct
association is not (dept (emp v)) but ((dept emp) v).
The LATERAL keyword, and the TABLE function mentioned in the
bug, are not the root cause, but they do make the problem
obvious because the wrong association causes a validation
error.
This commit fixes the precedence of the operators.
Close apache/calcite#3871
---
core/src/main/codegen/templates/Parser.jj | 39 ++++++-----
.../main/java/org/apache/calcite/sql/SqlJoin.java | 2 +-
.../org/apache/calcite/test/SqlValidatorTest.java | 32 +++++++++
core/src/test/resources/sql/lateral.iq | 77 ++++++++++++++++++++++
.../apache/calcite/sql/parser/SqlParserTest.java | 68 +++++++++++++++----
5 files changed, 186 insertions(+), 32 deletions(-)
diff --git a/core/src/main/codegen/templates/Parser.jj
b/core/src/main/codegen/templates/Parser.jj
index be994d260b..49ef4f6f1a 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)
- <COMMA> { 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)
+ <COMMA> { 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> ] // "LATERAL" is implicit with "UNNEST", so ignore
<UNNEST> { 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 ab08f2d1c9..6541afa385 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 @@ import static java.util.Objects.requireNonNull;
*/
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 2ff73fb2ed..fffb7591d0 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 @@ public class SqlValidatorTest extends
SqlValidatorTestCase {
.fails("Table 'DEPT' not found");
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-6266";>[CALCITE-6266]
+ * SqlValidatorException with LATERAL TABLE and JOIN</a>. */
+ @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 b38b0d7a7f..b4b9d3b952 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 5212736ebf..31b628b3b8 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.SqlCall;
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.apache.calcite.util.Util.toLinux;
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.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assumptions.assumeFalse;
+import static java.util.Objects.requireNonNull;
+
/**
* A <code>SqlParserTest</code> is a unit-test for
* {@link SqlParser the SQL parser}.
@@ -676,6 +680,19 @@ public class SqlParserTest {
};
}
+ /** 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<SqlNode> customMatches(String description,
+ Consumer<SqlNode> consumer) {
+ return new CustomTypeSafeMatcher<SqlNode>(description) {
+ @Override protected boolean matchesSafely(SqlNode sqlNode) {
+ consumer.accept(sqlNode);
+ return true;
+ }
+ };
+ }
+
protected SortedSet<String> getReservedKeywords() {
return keywords("c");
}
@@ -3203,6 +3220,28 @@ public class SqlParserTest {
+ "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 @@ public class SqlParserTest {
+ "Was expecting one of:\n"
+ " \"LATERAL\" \\.\\.\\.\n"
+ " \"TABLE\" \\.\\.\\.\n"
- + " \"UNNEST\" \\.\\.\\.\n"
+ " <IDENTIFIER> \\.\\.\\.\n"
+ " <HYPHENATED_IDENTIFIER> \\.\\.\\.\n"
+ " <QUOTED_IDENTIFIER> \\.\\.\\.\n"
@@ -4370,7 +4408,8 @@ public class SqlParserTest {
+ " <BIG_QUERY_BACK_QUOTED_IDENTIFIER> \\.\\.\\.\n"
+ " <BRACKET_QUOTED_IDENTIFIER> \\.\\.\\.\n"
+ " <UNICODE_QUOTED_IDENTIFIER> \\.\\.\\.\n"
- + " \"\\(\" \\.\\.\\.\n.*");
+ + " \"\\(\" \\.\\.\\.\n.*"
+ + " \"UNNEST\" \\.\\.\\.\n.*");
}
@Test void testEmptyValues() {
@@ -6061,8 +6100,8 @@ public class SqlParserTest {
+ "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 @@ public class SqlParserTest {
.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 @@ public class SqlParserTest {
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 @@ public class SqlParserTest {
+ "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.
