This is an automated email from the ASF dual-hosted git repository. mihaibudiu pushed a commit to branch issue7911 in repository https://gitbox.apache.org/repos/asf/calcite.git
commit d66099009a34b5d678515758ad8367206be0966e Author: Mihai Budiu <[email protected]> AuthorDate: Sat Oct 31 15:56:59 2026 -0700 [CALCITE-7911] FROM a, b JOIN c is parsed incorrectly Signed-off-by: Mihai Budiu <[email protected]> --- core/src/main/codegen/templates/Parser.jj | 42 ++++++----- .../main/java/org/apache/calcite/sql/SqlJoin.java | 2 +- .../calcite/sql/validate/SqlValidatorImpl.java | 13 +++- .../apache/calcite/sql2rel/RelDecorrelator.java | 36 +++++++-- .../apache/calcite/sql2rel/SqlToRelConverter.java | 69 ++++++++++++++++-- .../calcite/rel/rel2sql/RelToSqlConverterTest.java | 6 +- .../calcite/sql2rel/RelDecorrelatorTest.java | 34 +++++---- .../org/apache/calcite/test/SqlValidatorTest.java | 26 ++++++- .../apache/calcite/test/SqlToRelConverterTest.xml | 36 +++++---- core/src/test/resources/sql/join.iq | 85 ++++++++++++++++++++++ core/src/test/resources/sql/lateral.iq | 76 ++++++++++++++++++- site/_docs/reference.md | 6 ++ .../apache/calcite/sql/parser/SqlParserTest.java | 71 ++++++++++++++---- 13 files changed, 419 insertions(+), 83 deletions(-) diff --git a/core/src/main/codegen/templates/Parser.jj b/core/src/main/codegen/templates/Parser.jj index 6317b9c088..341920fd86 100644 --- a/core/src/main/codegen/templates/Parser.jj +++ b/core/src/main/codegen/templates/Parser.jj @@ -2309,39 +2309,43 @@ SqlNode FromClause() : SqlLiteral joinType; } { - e = TableRef1(ExprContext.ACCEPT_QUERY_OR_JOIN) + e = Join() ( // Comma joins should only occur at top-level in the FROM clause. + // JOIN binds tighter than comma, as required by the SQL standard: + // "FROM a, b JOIN c" means "FROM a, (b JOIN c)". // Valid: // * FROM a, b // * FROM (a CROSS JOIN b), c // Not valid: // * FROM a CROSS JOIN (b, c) - LOOKAHEAD(1) - e = JoinOrCommaTable(e) + LOOKAHEAD(2) + <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); + } )* { return e; } } -SqlNode JoinOrCommaTable(SqlNode e) : +/** Parses a table reference followed by zero or more joins. */ +SqlNode Join() : { - SqlNode e2; - SqlLiteral joinType; + SqlNode 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; } + e = TableRef1(ExprContext.ACCEPT_QUERY_OR_JOIN) + ( + LOOKAHEAD(2) + e = JoinTable(e) + )* + { return e; } } /** Matches "LEFT JOIN t ON ...", "RIGHT JOIN t USING ...", "JOIN t". */ 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 f948a7d668..7cb62854a5 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", 18); + new SqlJoinOperator("COMMA-JOIN", 16); public static final SqlJoinOperator OPERATOR = new SqlJoinOperator("JOIN", 18); 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 837d90b1d2..bfd8ee0970 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 @@ -2996,8 +2996,17 @@ private SqlNode registerFrom( } final SqlNode node2 = s != null ? s.getNode() : node; final TableScope tableScope = new TableScope(parentScope0, node2); - if (usingScope instanceof ListScope) { - for (ScopeChild child : ((ListScope) usingScope).children) { + // The items to the left of this one are visible, also those before an + // enclosing join, as "a" in "FROM a, LATERAL (...) CROSS JOIN b". + // JoinScope propagates each child to the enclosing JoinScope, so the + // outermost one holds all of them, in FROM-list order. + SqlValidatorScope outer = usingScope; + while (outer instanceof JoinScope + && ((JoinScope) outer).getUsingScope() instanceof JoinScope) { + outer = ((JoinScope) outer).getUsingScope(); + } + if (outer instanceof ListScope) { + for (ScopeChild child : ((ListScope) outer).children) { tableScope.addChild(child.namespace, child.name, child.nullable); } } diff --git a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java b/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java index 585b9d0196..0701e592ff 100644 --- a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java +++ b/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java @@ -1920,6 +1920,26 @@ private static boolean isWidening(RelDataType type, RelDataType type1) { assert rel.getRequiredColumns().cardinality() <= rightFrame.corDefOutputs.keySet().size(); + // In a LEFT correlate, the right input gives the values of the variables of + // enclosing correlates, and they are NULL in the rows that the join pads. + // Unless the parent supplies these NULLs again, get the values from the + // left input too; the conditions below match the copies. + Frame newLeftFrame = leftFrame; + if (rel.getJoinType().generatesNullsOnRight() + && !parentPropagatesNullValues) { + final List<CorRef> outerCorVars = new ArrayList<>(); + for (CorRef corRef : collectExternalCorVars(oldRight)) { + if (!corRef.corr.equals(rel.getCorrelationId())) { + outerCorVars.add(corRef); + } + } + if (!outerCorVars.isEmpty()) { + newLeftFrame = + supplyMissingCorVars(oldLeft, leftFrame, outerCorVars, + new TreeMap<>()); + } + } + // Change correlator rel into a join. // Join all the correlated variables produced by this correlator rel // with the values generated and propagated from the right input @@ -1927,7 +1947,7 @@ private static boolean isWidening(RelDataType type, RelDataType type1) { new TreeMap<>(rightFrame.corDefOutputs); final List<RexNode> conditions = new ArrayList<>(); final List<RelDataTypeField> newLeftOutput = - leftFrame.r.getRowType().getFieldList(); + newLeftFrame.r.getRowType().getFieldList(); int newLeftFieldCount = newLeftOutput.size(); final List<RelDataTypeField> newRightOutput = @@ -1939,7 +1959,7 @@ private static boolean isWidening(RelDataType type, RelDataType type1) { if (!corDef.corr.equals(rel.getCorrelationId())) { continue; } - final int newLeftPos = requireNonNull(leftFrame.oldToNewOutputs.get(corDef.field)); + final int newLeftPos = requireNonNull(newLeftFrame.oldToNewOutputs.get(corDef.field)); final int newRightPos = rightOutput.getValue(); // Using `equals` instead of `IS NOT DISTINCT FROM` is an optimization @@ -1960,6 +1980,12 @@ private static boolean isWidening(RelDataType type, RelDataType type1) { corDefOutputs.remove(corDef); } + // If both inputs propagate the same reference to an enclosing correlate, + // the two copies must have the same value. + conditions.addAll( + buildCorDefJoinConditions(newLeftFrame.corDefOutputs, corDefOutputs, + newLeftFrame.r, rightFrame.r, relBuilder)); + // Update the output position for the corVars: only pass on the cor // vars that are not used in the join key. for (Map.Entry<CorDef, Integer> entry : corDefOutputs.entrySet()) { @@ -1968,7 +1994,7 @@ private static boolean isWidening(RelDataType type, RelDataType type1) { // then add any corVar from the left input. Do not need to change // output positions. - corDefOutputs.putAll(leftFrame.corDefOutputs); + corDefOutputs.putAll(newLeftFrame.corDefOutputs); // Create the mapping between the output of the old correlation rel // and the new join rel @@ -1982,7 +2008,7 @@ private static boolean isWidening(RelDataType type, RelDataType type1) { == oldLeftFieldCount + oldRightFieldCount; // Left input positions are not changed. - mapOldToNewOutputs.putAll(leftFrame.oldToNewOutputs); + mapOldToNewOutputs.putAll(newLeftFrame.oldToNewOutputs); // Right input positions are shifted by newLeftFieldCount. for (int i = 0; i < oldRightFieldCount; i++) { @@ -1992,7 +2018,7 @@ private static boolean isWidening(RelDataType type, RelDataType type1) { final RexNode condition = RexUtil.composeConjunction(relBuilder.getRexBuilder(), conditions); - RelNode newJoin = relBuilder.push(leftFrame.r).push(rightFrame.r) + RelNode newJoin = relBuilder.push(newLeftFrame.r).push(rightFrame.r) .join(rel.getJoinType(), condition).build(); return register(rel, newJoin, mapOldToNewOutputs, corDefOutputs); 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 49953ced1f..e9fce1f55f 100644 --- a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java +++ b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java @@ -161,6 +161,7 @@ import org.apache.calcite.sql.validate.AggregatingSelectScope; import org.apache.calcite.sql.validate.CollectNamespace; import org.apache.calcite.sql.validate.DelegatingScope; +import org.apache.calcite.sql.validate.JoinScope; import org.apache.calcite.sql.validate.ListScope; import org.apache.calcite.sql.validate.MatchRecognizeScope; import org.apache.calcite.sql.validate.MeasureScope; @@ -3505,6 +3506,22 @@ protected RelNode createAsofJoin( return node; } + /** Returns the number of fields before a namespace in the FROM list of a + * scope, or 0 if the namespace is not in the list. */ + private static int fieldOffset(SqlValidatorScope scope, + SqlValidatorNamespace namespace) { + int offset = 0; + if (scope instanceof ListScope) { + for (SqlValidatorNamespace child : ((ListScope) scope).getChildren()) { + if (child == namespace) { + return offset; + } + offset += child.getRowType().getFieldCount(); + } + } + return 0; + } + private @Nullable CorrelationUse getCorrelationUse(Blackboard bb, final RelNode r0) { final Set<CorrelationId> correlatedVariables = RelOptUtil.getVariablesUsed(r0); @@ -3513,8 +3530,13 @@ protected RelNode createAsofJoin( } final ImmutableBitSet.Builder requiredColumns = ImmutableBitSet.builder(); final List<CorrelationId> correlNames = new ArrayList<>(); - // Mapping from (correlId, originalFieldIndex) to projectedFieldIndex for aggregation + // Mapping from (correlId, originalFieldIndex) to the new field index: the + // projected index for aggregation, or the index relative to the join inputs final Map<Pair<CorrelationId, Integer>, Integer> fieldMapping = new HashMap<>(); + // Offset in the FROM list of the first input of the join that defines the + // variable, and the row type of the variable, which spans the FROM list + int offset = 0; + @Nullable RelDataType correlRowType = null; for (CorrelationId correlName : correlatedVariables) { DeferredLookup lookup = @@ -3540,6 +3562,18 @@ protected RelNode createAsofJoin( continue; } + if (bb.scope instanceof JoinScope) { + // The variable belongs to the join whose inputs contain the item it + // references. These inputs need not start the FROM list, as + // "b CROSS JOIN LATERAL (... b.x ...)" in "FROM a, b CROSS JOIN ...". + final List<SqlValidatorNamespace> joinChildren = + ((JoinScope) bb.scope).getChildren(); + if (joinChildren.stream().noneMatch(ns -> ns == resolve.namespace)) { + continue; + } + offset = fieldOffset(ancestorScope, joinChildren.get(0)); + } + int namespaceOffset = 0; if (childNamespaceIndex > 0) { // If not the first child, need to figure out the width @@ -3585,10 +3619,11 @@ protected RelNode createAsofJoin( throw new AssertionError("Identifier '" + lookup.originalRelName + "." + originalFieldName + "' is not a group expr"); } - } + } // end for correlName - requiredColumns.set(pos); + requiredColumns.set(pos - offset); correlNames.add(correlName); + correlRowType = topLevelFieldAccess.getReferenceExpr().getType(); } if (correlNames.isEmpty()) { @@ -3607,13 +3642,28 @@ protected RelNode createAsofJoin( leaves.put(r, r.getRowType().getFieldCount()); } - // If there are field mappings (due to aggregation), rewrite the RelNode tree - // to update correlation variable row type and field indices + // A join scope never has an aggregate root + assert fieldMapping.isEmpty() || offset == 0 + : "aggregate root in a join scope"; + @Nullable RelDataType newCorrelRowType = null; if (!fieldMapping.isEmpty()) { + newCorrelRowType = bb.root().getRowType(); + } else if (offset > 0) { + final List<RelDataTypeField> fields = + requireNonNull(correlRowType, "correlRowType").getFieldList(); + for (int i = offset; i < fields.size(); i++) { + fieldMapping.put(Pair.of(correlNames.get(0), i), i - offset); + } + newCorrelRowType = + typeFactory.createStructType(fields.subList(offset, fields.size())); + } + if (newCorrelRowType != null) { + // Rewrite the RelNode tree to update the row type and field indices of + // the correlation variable r = r.accept( new CorrelationFieldMappingShuttle(rexBuilder, correlNames.get(0), - bb.root().getRowType(), fieldMapping)); + newCorrelRowType, fieldMapping)); } return new CorrelationUse(correlNames.get(0), requiredColumns.build(), r); @@ -6631,6 +6681,13 @@ private static class CorrelationFieldMappingShuttle extends RelHomogeneousShuttl } return super.visitFieldAccess(fieldAccess); } + + @Override public RexNode visitSubQuery(RexSubQuery subQuery) { + final RelNode r = + subQuery.rel.accept(CorrelationFieldMappingShuttle.this); + return super.visitSubQuery( + r == subQuery.rel ? subQuery : subQuery.clone(r)); + } }); } } diff --git a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java index 7d716d0947..bbaf33dd16 100644 --- a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java +++ b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java @@ -662,13 +662,13 @@ private static RelNode sqlToRel(String sql, SchemaPlus defaultSchema, + "match_condition \"employee\".\"department_id\" <= \"product\".\"product_id\"\n" + "on \"employee\".\"salary\" = \"product\".\"gross_weight\""; final String expected = "SELECT \"t\".\"full_name\"\nFROM " - + "((SELECT \"employee_id\", \"full_name\", \"first_name\", \"last_name\", \"position_id\", " + + "(SELECT \"employee_id\", \"full_name\", \"first_name\", \"last_name\", \"position_id\", " + "\"position_title\", \"store_id\", \"department_id\", \"birth_date\", \"hire_date\", " + "\"end_date\", \"salary\", \"supervisor_id\", \"education_level\", \"marital_status\", " + "\"gender\", \"management_role\", CAST(\"salary\" AS DOUBLE) AS \"salary0\"\n" - + "FROM \"foodmart\".\"employee\") AS \"t\" ASOF JOIN \"foodmart\".\"product\" " + + "FROM \"foodmart\".\"employee\") AS \"t\"\nASOF JOIN \"foodmart\".\"product\" " + "MATCH_CONDITION \"t\".\"department_id\" <= \"product\".\"product_id\" ON " - + "\"t\".\"salary0\" = \"product\".\"gross_weight\")"; + + "\"t\".\"salary0\" = \"product\".\"gross_weight\""; sql(sql).ok(expected); } diff --git a/core/src/test/java/org/apache/calcite/sql2rel/RelDecorrelatorTest.java b/core/src/test/java/org/apache/calcite/sql2rel/RelDecorrelatorTest.java index 2757a7a49b..d8f1fb54b2 100644 --- a/core/src/test/java/org/apache/calcite/sql2rel/RelDecorrelatorTest.java +++ b/core/src/test/java/org/apache/calcite/sql2rel/RelDecorrelatorTest.java @@ -287,13 +287,15 @@ public static Frameworks.ConfigBuilder config() { final String planAfter = "" + "LogicalSort(sort0=[$0], dir0=[ASC])\n" + " LogicalProject(ENAME_FROM_DEPT=[$2])\n" - + " LogicalJoin(condition=[IS NOT DISTINCT FROM($0, $1)], joinType=[left])\n" + + " LogicalJoin(condition=[=($0, $1)], joinType=[left])\n" + " LogicalValues(tuples=[[{ 7369 }, { 7499 }]])\n" + " LogicalAggregate(group=[{0}], agg#0=[SINGLE_VALUE($1)])\n" - + " LogicalProject(EMPNO1=[$11], EXPR$0=[||(||($1, ' from dept '), $12)])\n" - + " LogicalJoin(condition=[AND(=($7, $9), =($8, $10))], joinType=[left])\n" - + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], EMPNO0=[CAST($0):INTEGER NOT NULL])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n" + + " LogicalProject(EMPNO1=[$9], EXPR$0=[||(||($1, ' from dept '), $13)])\n" + + " LogicalJoin(condition=[AND(=($7, $10), =($8, $11), IS NOT DISTINCT FROM($9, $12))], joinType=[left])\n" + + " LogicalJoin(condition=[true], joinType=[inner])\n" + + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], EMPNO0=[CAST($0):INTEGER NOT NULL])\n" + + " LogicalTableScan(table=[[scott, EMP]])\n" + + " LogicalValues(tuples=[[{ 7369 }, { 7499 }]])\n" + " LogicalAggregate(group=[{0, 1, 2}], agg#0=[SINGLE_VALUE($3)])\n" + " LogicalProject(DEPTNO0=[$3], EMPNO0=[$4], EMPNO=[$5], DNAME=[$1])\n" + " LogicalJoin(condition=[=($0, $3)], joinType=[inner])\n" @@ -910,9 +912,12 @@ public static Frameworks.ConfigBuilder config() { + " LogicalProject(DEPTNO0=[$0], $f1=[true])\n" + " LogicalAggregate(group=[{0}])\n" + " LogicalProject(DEPTNO0=[$8])\n" - + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], DEPTNO0=[CAST($8):TINYINT], ENAME0=[$9], EXPR$0=[CAST($10):BIGINT])\n" - + " LogicalJoin(condition=[AND(IS NOT DISTINCT FROM($1, $9), =($7, $8))], joinType=[inner])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n" + + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], DEPTNO0=[$8], DEPTNO1=[CAST($9):TINYINT], ENAME0=[$10], EXPR$0=[CAST($11):BIGINT])\n" + + " LogicalJoin(condition=[AND(IS NOT DISTINCT FROM($1, $10), IS NOT DISTINCT FROM($8, $9))], joinType=[inner])\n" + + " LogicalJoin(condition=[=($7, $8)], joinType=[inner])\n" + + " LogicalTableScan(table=[[scott, EMP]])\n" + + " LogicalProject(DEPTNO=[$0])\n" + + " LogicalTableScan(table=[[scott, DEPT]])\n" + " LogicalFilter(condition=[>($2, 5)])\n" + " LogicalAggregate(group=[{0, 1}], EXPR$0=[SUM($2)])\n" + " LogicalProject(DEPTNO=[$0], ENAME=[$1], X=[$2])\n" @@ -1025,14 +1030,17 @@ public static Frameworks.ConfigBuilder config() { // Verify plan final String planAfter = "" + "LogicalProject(DNAME=[$1])\n" - + " LogicalJoin(condition=[IS NOT DISTINCT FROM($0, $3)], joinType=[inner])\n" + + " LogicalJoin(condition=[=($0, $3)], joinType=[inner])\n" + " LogicalTableScan(table=[[scott, DEPT]])\n" + " LogicalProject(DEPTNO0=[$0], $f1=[true])\n" + " LogicalAggregate(group=[{0}])\n" - + " LogicalProject(DEPTNO0=[$9])\n" - + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], ENAME0=[$8], DEPTNO0=[CAST($9):TINYINT], EXPR$0=[CAST($10):BIGINT])\n" - + " LogicalJoin(condition=[IS NOT DISTINCT FROM($1, $8)], joinType=[inner])\n" - + " LogicalTableScan(table=[[scott, EMP]])\n" + + " LogicalProject(DEPTNO0=[$8])\n" + + " LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], DEPTNO0=[$8], ENAME0=[$9], DEPTNO1=[CAST($10):TINYINT], EXPR$0=[CAST($11):BIGINT])\n" + + " LogicalJoin(condition=[AND(IS NOT DISTINCT FROM($1, $9), IS NOT DISTINCT FROM($8, $10))], joinType=[inner])\n" + + " LogicalJoin(condition=[true], joinType=[inner])\n" + + " LogicalTableScan(table=[[scott, EMP]])\n" + + " LogicalProject(DEPTNO=[$0])\n" + + " LogicalTableScan(table=[[scott, DEPT]])\n" + " LogicalFilter(condition=[>($2, 5)])\n" + " LogicalAggregate(group=[{0, 1}], EXPR$0=[SUM($2)])\n" + " LogicalProject(ENAME=[$0], DEPTNO=[$1], X=[$2])\n" 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 ba5d9b02de..db035bbfb6 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java @@ -10765,9 +10765,8 @@ void testOrderByAliasPreservesValidatedExpressions(String query) { * <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". + // CROSS JOIN binds tighter than ",", so "table(ramp(deptno))" is joined + // with "values"; LATERAL still sees "dept", which is before the comma. sql("select *\n" + "from dept,\n" + " lateral table(ramp(deptno))\n" @@ -10791,6 +10790,27 @@ void testOrderByAliasPreservesValidatedExpressions(String query) { + " lateral (select * from dept where dept.deptno = emp.deptno),\n" + " ^emp^") .fails("Duplicate relation name 'EMP' in FROM clause"); + // LATERAL on the right of a join sees the items on the left of the join + // and the items before the comma + sql("select *\n" + + "from dept,\n" + + " emp cross join lateral (select * from bonus\n" + + " where bonus.ename = emp.ename and bonus.job = dept.name)") + .ok(); + } + + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-7911";>[CALCITE-7911] + * FROM a, b JOIN c is parsed incorrectly</a>. */ + @Test void testJoinOnAfterComma() { + // "emp, dept join bonus on c" means "emp, (dept join bonus on c)", + // so c sees dept and bonus, but not emp + sql("select *\n" + + "from emp, dept join bonus on dept.name = bonus.job") + .ok(); + sql("select *\n" + + "from emp, dept join bonus on ^emp^.ename = bonus.ename") + .fails("Table 'EMP' not found"); } /** Test case for diff --git a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml index 22aeace682..88bde935de 100644 --- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml @@ -6576,17 +6576,21 @@ from (select 2+deptno d2, 3+deptno d3 from emp) e <Resource name="plan"> <![CDATA[ LogicalProject(D2=[$0], D3=[$1]) - LogicalProject(D2=[$0], D3=[$1], D1=[CAST($2):INTEGER], D6=[$3], $f2=[CAST($4):BOOLEAN]) - LogicalJoin(condition=[AND(=($0, $2), IS NOT DISTINCT FROM($1, $3))], joinType=[inner]) + LogicalProject(D2=[$0], D3=[$1], D1=[CAST($2):INTEGER], D30=[CAST($3):INTEGER], $f2=[CAST($4):BOOLEAN]) + LogicalJoin(condition=[AND(=($0, $2), =($1, $3))], joinType=[inner]) LogicalProject(D2=[+(2, $7)], D3=[+(3, $7)]) LogicalTableScan(table=[[CATALOG, SALES, EMP]]) LogicalAggregate(group=[{0, 1}], agg#0=[MIN($2)]) - LogicalProject(D1=[$0], D6=[$2], $f0=[true]) + LogicalProject(D1=[$0], D3=[$2], $f0=[true]) LogicalFilter(condition=[IS NOT NULL($1)]) - LogicalProject(D1=[$0], $f0=[$3], D6=[$2]) - LogicalJoin(condition=[=($0, $1)], joinType=[left]) - LogicalProject(D1=[+($0, 1)]) - LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) + LogicalProject(D1=[$0], $f0=[$4], D3=[$1]) + LogicalJoin(condition=[AND(=($0, $2), IS NOT DISTINCT FROM($1, $3))], joinType=[left]) + LogicalJoin(condition=[true], joinType=[inner]) + LogicalProject(D1=[+($0, 1)]) + LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) + LogicalAggregate(group=[{0}]) + LogicalProject(D3=[+(3, $7)]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) LogicalAggregate(group=[{0, 1}], agg#0=[MIN($2)]) LogicalProject(D4=[$0], D6=[$2], $f0=[true]) LogicalFilter(condition=[=($1, $0)]) @@ -6606,17 +6610,21 @@ from (select 2+deptno d2, 3+deptno d3 from emp) e <Resource name="plan"> <![CDATA[ LogicalProject(D2=[$0], D3=[$1]) - LogicalProject(D2=[$0], D3=[$1], D1=[CAST($2):INTEGER], D6=[$3], $f2=[CAST($4):BOOLEAN]) - LogicalJoin(condition=[AND(=($0, $2), IS NOT DISTINCT FROM($1, $3))], joinType=[inner]) + LogicalProject(D2=[$0], D3=[$1], D1=[CAST($2):INTEGER], D30=[CAST($3):INTEGER], $f2=[CAST($4):BOOLEAN]) + LogicalJoin(condition=[AND(=($0, $2), =($1, $3))], joinType=[inner]) LogicalProject(D2=[+(2, $7)], D3=[+(3, $7)]) LogicalTableScan(table=[[CATALOG, SALES, EMP]]) LogicalAggregate(group=[{0, 1}], agg#0=[MIN($2)]) - LogicalProject(D1=[$0], D6=[$2], $f0=[true]) + LogicalProject(D1=[$0], D3=[$2], $f0=[true]) LogicalFilter(condition=[IS NOT NULL($1)]) - LogicalProject(D1=[$0], $f0=[$3], D6=[$2]) - LogicalJoin(condition=[=($0, $1)], joinType=[left]) - LogicalProject(D1=[+($0, 1)]) - LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) + LogicalProject(D1=[$0], $f0=[$4], D3=[$1]) + LogicalJoin(condition=[AND(=($0, $2), IS NOT DISTINCT FROM($1, $3))], joinType=[left]) + LogicalJoin(condition=[true], joinType=[inner]) + LogicalProject(D1=[+($0, 1)]) + LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) + LogicalAggregate(group=[{0}]) + LogicalProject(D3=[+(3, $7)]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) LogicalAggregate(group=[{0, 1}], agg#0=[MIN($2)]) LogicalProject(D4=[$0], D6=[$2], $f0=[true]) LogicalFilter(condition=[=($1, $0)]) diff --git a/core/src/test/resources/sql/join.iq b/core/src/test/resources/sql/join.iq index bac15e2a08..de770cced9 100644 --- a/core/src/test/resources/sql/join.iq +++ b/core/src/test/resources/sql/join.iq @@ -91,6 +91,91 @@ where t1.i + t2.i + t3.i + t4.i = 1; !ok +# [CALCITE-7911] FROM a, b JOIN c is parsed incorrectly +# JOIN binds tighter than comma: s CROSS JOIN (t RIGHT JOIN u ON t.x = u.y) +# Same result on PostgreSQL 14. +with s (c0) as (values 'a', 'b'), + t (x) as (values 1, 2), + u (y) as (values 10, 20) +select s.c0, t.x, u.y +from s, t right join u on t.x = u.y; ++----+---+----+ +| C0 | X | Y | ++----+---+----+ +| a | | 10 | +| a | | 20 | +| b | | 10 | +| b | | 20 | ++----+---+----+ +(4 rows) + +!ok + +# s CROSS JOIN (u LEFT JOIN t ON t.x = u.y) +# Same result on PostgreSQL 14. +with s (c0) as (values 'a', 'b'), + t (x) as (values 1, 2), + u (y) as (values 10, 20) +select s.c0, t.x, u.y +from s, u left join t on t.x = u.y; ++----+---+----+ +| C0 | X | Y | ++----+---+----+ +| a | | 10 | +| a | | 20 | +| b | | 10 | +| b | | 20 | ++----+---+----+ +(4 rows) + +!ok + +# s CROSS JOIN (t FULL JOIN u ON t.x = u.y) +# Same result on PostgreSQL 14. +with s (c0) as (values 'a', 'b'), + t (x) as (values 1, 2), + u (y) as (values 10, 20) +select s.c0, t.x, u.y +from s, t full join u on t.x = u.y; ++----+---+----+ +| C0 | X | Y | ++----+---+----+ +| a | 1 | | +| a | 2 | | +| a | | 10 | +| a | | 20 | +| b | 1 | | +| b | 2 | | +| b | | 10 | +| b | | 20 | ++----+---+----+ +(8 rows) + +!ok + +# Correlated sub-query in the ON clause of a join after a comma. +# Same result on PostgreSQL 14. +with emp (ename, deptno, sal) + as (values ('SMITH', 20, 800), ('CLARK', 10, 2450), + ('ALLEN', 30, 1600), ('JONES', 20, 2975)), + dept (deptno) as (values (10), (20)) +select v.v, e.ename, d.deptno +from (values ('A'), ('M')) as v (v), + emp as e join dept as d + on d.deptno = e.deptno + and e.sal = (select max(sal) from emp as e2 where e2.deptno = d.deptno); ++---+-------+--------+ +| V | ENAME | DEPTNO | ++---+-------+--------+ +| A | CLARK | 10 | +| A | JONES | 20 | +| M | CLARK | 10 | +| M | JONES | 20 | ++---+-------+--------+ +(4 rows) + +!ok + # OR is a nestedLoop join select * from emp diff --git a/core/src/test/resources/sql/lateral.iq b/core/src/test/resources/sql/lateral.iq index f560c78d67..a7030af453 100644 --- a/core/src/test/resources/sql/lateral.iq +++ b/core/src/test/resources/sql/lateral.iq @@ -152,9 +152,9 @@ from dept, !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. +# The problem also occurred in ', LATERAL (SELECT ...) JOIN'. 'JOIN' binds +# tighter than ',', so the LATERAL item is inside a join, and it must see +# the items before the ','. Same result on PostgreSQL 14. with dept (deptno, dname) as (values (10, 'ACCOUNTING'), (20,'RESEARCH')), emp (empno, deptno, ename) @@ -175,6 +175,76 @@ from dept, !ok +# LATERAL on the right of a join sees the item on the left of the join (v) +# and the item before the ',' (dept). Same result on PostgreSQL 14. +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, + (values 'A', 'M') as v (v) + cross join lateral (select * from emp + where emp.deptno = dept.deptno + and position(v.v in emp.ename) > 0) as emp; ++--------+------------+---+-------+---------+-------+ +| DEPTNO | DNAME | V | EMPNO | DEPTNO0 | ENAME | ++--------+------------+---+-------+---------+-------+ +| 10 | ACCOUNTING | A | 7782 | 10 | CLARK | +| 20 | RESEARCH | M | 7369 | 20 | SMITH | ++--------+------------+---+-------+---------+-------+ +(2 rows) + +!ok + +# As above, with LEFT JOIN; the rows that the join pads keep the value of +# "dept". Same result on PostgreSQL 14. +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 dept.deptno, emp.ename, v.v +from dept, + (values ('A'), ('M')) as v (v) + left join lateral (select * from emp + where emp.deptno = dept.deptno + and position(v.v in emp.ename) > 0) as emp on true; ++--------+-------+---+ +| DEPTNO | ENAME | V | ++--------+-------+---+ +| 10 | CLARK | A | +| 10 | | M | +| 20 | SMITH | M | +| 20 | | A | ++--------+-------+---+ +(4 rows) + +!ok + +# LATERAL sees two items before the ',' (v1 and dept) and the item on the +# left of the join (v2). Same result on PostgreSQL 14. +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 v1.v as v1, dept.deptno, emp.ename, v2.v as v2 +from (values ('A'), ('M')) as v1 (v), + dept, + (values ('A'), ('M')) as v2 (v) + cross join lateral (select * from emp + where emp.deptno = dept.deptno + and position(v2.v in emp.ename) > 0 + and v1.v = 'A') as emp; ++----+--------+-------+----+ +| V1 | DEPTNO | ENAME | V2 | ++----+--------+-------+----+ +| A | 10 | CLARK | A | +| A | 20 | SMITH | M | ++----+--------+-------+----+ +(2 rows) + +!ok + # UNNEST applied to VALUES with t (x, ys) as (values (1, array [2,3]), (4, array [5])) diff --git a/site/_docs/reference.md b/site/_docs/reference.md index 889645ae17..ed32963b2f 100644 --- a/site/_docs/reference.md +++ b/site/_docs/reference.md @@ -429,6 +429,12 @@ ## Grammar in those same conformance levels, any *column* in *insert* may be replaced by *columnDecl*, which has a similar effect to including it in an EXTEND clause. +In *tableExpression*, JOIN binds tighter than comma, as in the SQL standard: +"FROM a, b JOIN c ON c.x = b.x" is equivalent to +"FROM a, (b JOIN c ON c.x = b.x)", so the ON condition can reference +`b` and `c` but not `a`. A LATERAL item can reference the items to its left, +also those before a comma. + In *orderItem*, if *expression* is a positive integer *n*, it denotes the <em>n</em>th item in the SELECT clause. 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 952a2c97cf..88105a43cc 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 @@ -16,6 +16,7 @@ */ package org.apache.calcite.sql.parser; import org.apache.calcite.avatica.util.Quoting; +import org.apache.calcite.sql.JoinType; import org.apache.calcite.sql.SqlCall; import org.apache.calcite.sql.SqlDelete; import org.apache.calcite.sql.SqlDialect; @@ -3632,19 +3633,61 @@ void checkPeriodPredicate(Checker checker) { 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.) + // JOIN binds tighter than comma: + // select * from a as a2, (b cross join c) 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)); + assertThat(from.getJoinType(), is(JoinType.COMMA)); + assertThat(from.getLeft().getKind(), is(SqlKind.AS)); + assertThat(from.getRight(), instanceOf(SqlJoin.class)); })); } + private static String joinTree(SqlNode node) { + if (node instanceof SqlJoin) { + final SqlJoin join = (SqlJoin) node; + return join.getJoinType() + "(" + joinTree(join.getLeft()) + ", " + + joinTree(join.getRight()) + ")"; + } + return node.toString(); + } + + /** Returns a matcher for a query whose FROM clause has a given join tree; + * for example "COMMA(A, RIGHT(B, C))" for "FROM a, b RIGHT JOIN c". */ + private static Matcher<SqlNode> hasJoinTree(String expected) { + return customMatches(expected, node -> { + final SqlSelect select = (SqlSelect) node; + assertThat(joinTree(requireNonNull(select.getFrom())), is(expected)); + }); + } + + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-7911";>[CALCITE-7911] + * FROM a, b JOIN c is parsed incorrectly</a>. */ + @Test void testJoinCommaPrecedence() { + // An explicit JOIN binds tighter than comma; joins and commas are each + // left-associative. + sql("select * from a, b right join c on b.x = c.x") + .node(hasJoinTree("COMMA(A, RIGHT(B, C))")); + sql("select * from a, b join c on true, d left join e on true") + .node(hasJoinTree("COMMA(COMMA(A, INNER(B, C)), LEFT(D, E))")); + sql("select * from a join b on true join c on true, d") + .node(hasJoinTree("COMMA(INNER(INNER(A, B), C), D)")); + // Printed SQL must parse to the same tree. + final String expected = "SELECT *\n" + + "FROM `A`,\n" + + "`B`\n" + + "RIGHT JOIN `C` ON (`B`.`X` = `C`.`X`)"; + sql("select * from a, (b right join c on b.x = c.x)") + .ok(expected) + .node(hasJoinTree("COMMA(A, RIGHT(B, C))")); + sql(expected) + .withConfig(c -> c.withQuoting(Quoting.BACK_TICK)) + .node(hasJoinTree("COMMA(A, RIGHT(B, C))")); + } + @Test void testInternalComma() { sql("select * from (a^,^ b) cross join c") .fails("(?s)Encountered \",\" at .*"); @@ -5071,19 +5114,19 @@ void checkPeriodPredicate(Checker checker) { + "match_condition orders.ts <= products.expiry\n" + "on orders.productid = products.productid"; final String expected0 = "SELECT *\n" - + "FROM (`ORDERS` " + + "FROM `ORDERS`\n" + "ASOF JOIN `PRODUCTS` " + "MATCH_CONDITION (`ORDERS`.`TS` <= `PRODUCTS`.`EXPIRY`) " - + "ON (`ORDERS`.`PRODUCTID` = `PRODUCTS`.`PRODUCTID`))"; + + "ON (`ORDERS`.`PRODUCTID` = `PRODUCTS`.`PRODUCTID`)"; sql(sql0).ok(expected0); final String sql1 = "select * from orders left asof join products\n" + "match_condition orders.ts <= products.expiry\n" + "on orders.productid = products.productid"; final String expected1 = "SELECT *\n" - + "FROM (`ORDERS` " + + "FROM `ORDERS`\n" + "LEFT ASOF JOIN `PRODUCTS` " + "MATCH_CONDITION (`ORDERS`.`TS` <= `PRODUCTS`.`EXPIRY`) " - + "ON (`ORDERS`.`PRODUCTID` = `PRODUCTS`.`PRODUCTID`))"; + + "ON (`ORDERS`.`PRODUCTID` = `PRODUCTS`.`PRODUCTID`)"; sql(sql1).ok(expected1); sql("select * from orders asof join products\n" @@ -6869,8 +6912,8 @@ private static Matcher<SqlNode> 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)Encountered \", SELECT\" at.*"); + 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(1, ^SELECT^ x FROM (VALUES(1)) x)") .fails("(?s)Incorrect syntax near the keyword 'SELECT'.*"); } @@ -7009,8 +7052,8 @@ private static Matcher<SqlNode> 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)Encountered \", SELECT\" at.*"); + sql("SELECT map(SELECT x FROM (VALUES(1)) x, ^SELECT^ x FROM (VALUES(1)) x)") + .fails("(?s)Incorrect syntax near the keyword 'SELECT' at.*"); } @Test void testVisitSqlInsertWithSqlShuttle() {
