Skip to content
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
4 changes: 1 addition & 3 deletions core/src/main/codegen/templates/Parser.jj
Original file line number Diff line number Diff line change
Expand Up @@ -2516,9 +2516,7 @@ SqlNode TableRef3(ExprContext exprContext, boolean lateral) :
// Standard SQL (and Postgres) allow applying "AS alias" to a JOIN,
// e.g. "FROM (a CROSS JOIN b) AS c". The new alias obscures the
// internal aliases, and columns cannot be referenced if they are
// not unique. TODO: Support this behavior; see
// [CALCITE-5168] Allow AS after parenthesized JOIN
checkNotJoin(tableRef);
// not unique.
if (columnAliasList == null) {
tableRef = SqlStdOperatorTable.AS.createCall(
Span.of(tableRef).end(this), tableRef, alias);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.apache.calcite.rel.type.RelDataTypeFactoryImpl;
import org.apache.calcite.rel.type.RelDataTypeField;
import org.apache.calcite.rel.type.SingleColumnAliasRelDataType;
import org.apache.calcite.rel.type.StructKind;
import org.apache.calcite.sql.SqlBasicCall;
import org.apache.calcite.sql.SqlCall;
import org.apache.calcite.sql.SqlIdentifier;
Expand Down Expand Up @@ -82,7 +83,20 @@ protected AliasNamespace(
final List<SqlNode> operands = call.getOperandList();
final SqlValidatorNamespace childNs =
validator.getNamespaceOrThrow(operands.get(0));
final RelDataType rowType = childNs.getRowTypeSansSystemColumns();
final RelDataType rowType0 = childNs.getRowTypeSansSystemColumns();
final RelDataType rowType;
if (rowType0.isStruct()) {
rowType = rowType0;
} else {
// Joins produce RelCrossType, which is not a struct. Convert to a struct
// so that columns can be resolved via the alias.
rowType = validator.getTypeFactory().builder()
.kind(StructKind.FULLY_QUALIFIED)
.addAll(
Util.transform(rowType0.getFieldList(),
f -> Pair.of(f.getName(), f.getType())))
.build();
}
final RelDataType aliasedType;
if (operands.size() == 2) {
final SqlNode node = operands.get(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2864,11 +2864,17 @@ private SqlNode registerFrom(
expr = call.operand(0);
final boolean needAliasNamespace = call.operandCount() > 2
|| expr.getKind() == SqlKind.VALUES || expr.getKind() == SqlKind.UNNEST
|| expr.getKind() == SqlKind.COLLECTION_TABLE;
|| expr.getKind() == SqlKind.COLLECTION_TABLE
|| expr.getKind() == SqlKind.JOIN;
// For an aliased join, the join's children must not be visible outside
// the alias. Prevent JoinScope.addChild from propagating children to
// the using scope by using parentScope.
final SqlValidatorScope exprUsingScope =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is subtle, but the tests pass, so I hope it's right

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks, agreed this is subtle. The intent is to prevent the children of an aliased join from leaking into the outer scope; the new validator tests cover that emp/bonus are no longer visible once the join is aliased as x.

expr.getKind() == SqlKind.JOIN ? parentScope : usingScope;
newExpr =
registerFrom(
parentScope,
usingScope,
exprUsingScope,
!needAliasNamespace,
expr,
enclosingNode,
Expand Down
17 changes: 11 additions & 6 deletions core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -6625,12 +6625,17 @@ void testReturnsCorrectRowTypeOnCombinedJoin() {
sql("select * from (emp join bonus using (job))\n"
+ "join dept using (deptno)").ok();

// Cannot alias a JOIN (until
// [CALCITE-5168] Allow AS after parenthesized JOIN
// is fixed).
sql("select * from (emp ^join^ bonus using (job)) as x\n"
+ "join dept using (deptno)")
.fails("Join expression encountered in illegal context");
// [CALCITE-5168] Allow AS after parenthesized JOIN.
sql("select x.empno from (emp cross join dept) as x").ok();
sql("select x.empno from (emp join bonus using (job)) as x").ok();
sql("select x.a from ((select empno from emp) cross join "
+ "(select deptno from dept)) as x (a, c)")
.ok();
// Inner aliases are obscured by the new alias.
sql("select ^emp^.empno from (emp cross join dept) as x")
.fails("Table 'EMP' not found");
sql("select ^bonus^.job from (emp join bonus using (job)) as x")
.fails("Table 'BONUS' not found");
sql("select * from (emp join bonus using (job))\n"
+ "join dept using (^dname^)")
.fails("Column 'DNAME' not found in any table");
Expand Down
17 changes: 17 additions & 0 deletions core/src/test/resources/sql/join.iq
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,23 @@ cross join (bonus as b

!ok

# [CALCITE-5168] Allow AS after parenthesized JOIN
select d.dname, j.empno, j.ename

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This sql programe was validated in postgresql: https://onecompiler.com/postgresql/44ypqwt2u

from dept as d
cross join (emp as e cross join (values (1)) as b(dummy)) as j
where j.empno = 7369;
+------------+-------+-------+
| DNAME | EMPNO | ENAME |
+------------+-------+-------+
| ACCOUNTING | 7369 | SMITH |
| RESEARCH | 7369 | SMITH |
| SALES | 7369 | SMITH |
| OPERATIONS | 7369 | SMITH |
+------------+-------+-------+
(4 rows)

!ok

# Join plus TABLE
select e.ename, d.dname
from emp as e
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7860,9 +7860,9 @@ private static Consumer<List<? extends Throwable>> checkWarnings(
// is syntactically and semantically valid; but
// "select t.i from (t cross join u) as x"
// is semantically invalid.
// TODO: Support this in Calcite.
sql("select * from (t cross ^join^ u) as x")
.fails("Join expression encountered in illegal context");
sql("select * from (t cross join u) as x")
.ok("SELECT *\n"
+ "FROM (`T` CROSS JOIN `U`) AS `X`");
sql("select *\n"
+ "from (t cross ^join^ u)\n"
+ " tablesample substitute('medium')")
Expand Down Expand Up @@ -8026,21 +8026,24 @@ private static Consumer<List<? extends Throwable>> checkWarnings(
+ "CROSS JOIN (TABLE `T2`)";
sql(sql3).ok(expected3);

// Adding an alias to the previous query makes it invalid
// (The error message and location could be improved)
final String expected4 = "SELECT *\n"
+ "FROM ((SELECT *\n"
+ "FROM `T`) CROSS JOIN (TABLE `T2`)) AS `X`";
final String sql4 = "SELECT *\n"
+ "FROM ((((((((((((SELECT * FROM t)))\n"
+ " cross ^join^ ((table t2))))))))))) X";
+ " cross join ((table t2))))))))))) X";
final String sql5 = "SELECT *\n"
+ "FROM ((((((((((((SELECT * FROM t)))\n"
+ " cross ^join^ ((table t2))))))))))) as X";
+ " cross join ((table t2))))))))))) as X";
final String sql6 = "SELECT *\n"
+ "FROM ((((((((((((SELECT * FROM t)))\n"
+ " cross ^join^ ((table t2))))))))))) as X (a, b, c)";
final String message = "Join expression encountered in illegal context";
sql(sql4).fails(message);
sql(sql5).fails(message);
sql(sql6).fails(message);
+ " cross join ((table t2))))))))))) as X (a, b, c)";
sql(sql4).ok(expected4);
sql(sql5).ok(expected4);
final String expected6 = "SELECT *\n"
+ "FROM ((SELECT *\n"
+ "FROM `T`) CROSS JOIN (TABLE `T2`)) AS `X` (`A`, `B`, `C`)";
sql(sql6).ok(expected6);
}

@Test void testProcedureCall() {
Expand Down
Loading