Skip to content

Commit 85d0bdd

Browse files
committed
[CALCITE-7663] RelToSqlConverter generates ambiguous column references when expanding SELECT * over a join with duplicate field names
When a dialect's supportGenerateSelectStar() returns false for a join with duplicate field names, the SELECT * expansion in SqlImplementor did not alias the expanded columns to their unique row-type field names. A sub-query wrapping such a join then exposed two identically named columns, making outer references ambiguous (e.g. PostgreSQL: column reference "id" is ambiguous). Alias each expanded column to its unique row-type field name, mirroring the validator path.
1 parent 85b042c commit 85d0bdd

2 files changed

Lines changed: 72 additions & 3 deletions

File tree

core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,18 @@ protected SqlCall as(SqlNode e, String alias, String... fieldNames) {
296296
return SqlStdOperatorTable.AS.createCall(POS, operandList);
297297
}
298298

299+
/** Wraps a column reference in an {@code AS} alias when its intrinsic name
300+
* differs from {@code name}, so that the column is emitted with
301+
* {@code name}. */
302+
private SqlNode renameAs(SqlNode fieldNode, String name) {
303+
final String currentName = fieldNode instanceof SqlIdentifier
304+
? Util.last(((SqlIdentifier) fieldNode).names)
305+
: null;
306+
return name.equals(currentName)
307+
? fieldNode
308+
: as(fieldNode, name);
309+
}
310+
299311
/** Returns whether a list of expressions projects all fields, in order,
300312
* from the input, with the same names. */
301313
public static boolean isStar(List<RexNode> exps, RelDataType inputRowType,
@@ -2029,9 +2041,17 @@ private Builder builder(RelNode rel, Set<Clause> clauses) {
20292041
newContext = aliasContext(aliases, qualified);
20302042
}
20312043
if (!dialect.supportGenerateSelectStar(rel.getInput(0))) {
2044+
// Rename each expanded column to its (unique) row-type field name.
2045+
// Otherwise a sub-query that wraps a join with duplicate field names
2046+
// (e.g. two columns named DEPTNO) would expose two identically named
2047+
// columns, which is ambiguous when referenced from an outer query.
2048+
final List<String> fieldNames = rel.getRowType().getFieldNames();
20322049
final List<SqlNode> expandedSelectList = new ArrayList<>();
20332050
for (int i = 0; i < newContext.fieldCount; i++) {
2034-
expandedSelectList.add(newContext.field(i));
2051+
final SqlNode field = newContext.field(i);
2052+
expandedSelectList.add(i < fieldNames.size()
2053+
? renameAs(field, fieldNames.get(i))
2054+
: field);
20352055
}
20362056
select.setSelectList(new SqlNodeList(expandedSelectList, POS));
20372057
}
@@ -2392,9 +2412,17 @@ SqlSelect maybeExpandStar(SqlSelect select) {
23922412
boolean qualified =
23932413
!dialect.hasImplicitTableAlias() || aliases.size() > 1;
23942414
final Context ctx = aliasContext(aliases, qualified);
2415+
// Rename each expanded column to its (unique) row-type field name.
2416+
// Otherwise a sub-query that wraps a join with duplicate field names
2417+
// (e.g. two columns named DEPTNO) would expose two identically named
2418+
// columns, which is ambiguous when referenced from an outer query.
2419+
final List<String> fieldNames = expectedRel.getRowType().getFieldNames();
23952420
final List<SqlNode> expandedList = new ArrayList<>();
23962421
for (int i = 0; i < ctx.fieldCount; i++) {
2397-
expandedList.add(ctx.field(i));
2422+
final SqlNode field = ctx.field(i);
2423+
expandedList.add(i < fieldNames.size()
2424+
? renameAs(field, fieldNames.get(i))
2425+
: field);
23982426
}
23992427
return new SqlSelect(select.getParserPosition(),
24002428
(SqlNodeList) select.getOperandList().get(0),

core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9669,18 +9669,59 @@ private void checkLiteral2(String expression, String expected) {
96699669
b.equals(b.field(2, 0, "DEPTNO"),
96709670
b.field(2, 1, "DEPTNO")))
96719671
.build();
9672+
// The join has two columns named DEPTNO; the second is aliased to its
9673+
// unique row-type field name (DEPTNO0) so the result never exposes two
9674+
// identically named columns (CALCITE-7663).
96729675
final String expected = "SELECT"
96739676
+ " \"EMP\".\"EMPNO\", \"EMP\".\"ENAME\", \"EMP\".\"JOB\","
96749677
+ " \"EMP\".\"MGR\", \"EMP\".\"HIREDATE\", \"EMP\".\"SAL\","
96759678
+ " \"EMP\".\"COMM\", \"EMP\".\"DEPTNO\","
9676-
+ " \"DEPT\".\"DEPTNO\","
9679+
+ " \"DEPT\".\"DEPTNO\" AS \"DEPTNO0\","
96779680
+ " \"DEPT\".\"DNAME\", \"DEPT\".\"LOC\"\n"
96789681
+ "FROM \"scott\".\"EMP\"\n"
96799682
+ "INNER JOIN \"scott\".\"DEPT\""
96809683
+ " ON \"EMP\".\"DEPTNO\" = \"DEPT\".\"DEPTNO\"";
96819684
relFn(relFn).dialect(NO_STAR_DIALECT).ok(expected);
96829685
}
96839686

9687+
/** Test case for
9688+
* <a href="https://issues.apache.org/jira/browse/CALCITE-7663">[CALCITE-7663]</a>.
9689+
* A join with duplicate field names (two DEPTNO) wrapped by a FETCH becomes a
9690+
* sub-query; when it is joined again, the sub-query must not expose two
9691+
* columns with the same name, otherwise the outer references to them are
9692+
* ambiguous (e.g. PostgreSQL: {@code column reference "deptno" is ambiguous}).
9693+
* Each expanded column is aliased to its unique row-type field name. */
9694+
@Test void testNoSelectStarJoinWithDuplicateNamesAndFetchIsNotAmbiguous() {
9695+
final Function<RelBuilder, RelNode> relFn = b -> b
9696+
.scan("EMP")
9697+
.scan("DEPT")
9698+
.join(JoinRelType.INNER,
9699+
b.equals(b.field(2, 0, "DEPTNO"), b.field(2, 1, "DEPTNO")))
9700+
.limit(0, 10)
9701+
.scan("DEPT")
9702+
.join(JoinRelType.INNER,
9703+
b.equals(b.field(2, 0, "EMPNO"), b.field(2, 1, "DEPTNO")))
9704+
.limit(0, 5)
9705+
.build();
9706+
final String expected = "SELECT \"t\".\"EMPNO\", \"t\".\"ENAME\","
9707+
+ " \"t\".\"JOB\", \"t\".\"MGR\", \"t\".\"HIREDATE\", \"t\".\"SAL\","
9708+
+ " \"t\".\"COMM\", \"t\".\"DEPTNO\", \"t\".\"DEPTNO0\","
9709+
+ " \"t\".\"DNAME\", \"t\".\"LOC\","
9710+
+ " \"DEPT0\".\"DEPTNO\" AS \"DEPTNO1\","
9711+
+ " \"DEPT0\".\"DNAME\" AS \"DNAME0\", \"DEPT0\".\"LOC\" AS \"LOC0\"\n"
9712+
+ "FROM (SELECT \"EMP\".\"EMPNO\", \"EMP\".\"ENAME\", \"EMP\".\"JOB\","
9713+
+ " \"EMP\".\"MGR\", \"EMP\".\"HIREDATE\", \"EMP\".\"SAL\","
9714+
+ " \"EMP\".\"COMM\", \"EMP\".\"DEPTNO\","
9715+
+ " \"DEPT\".\"DEPTNO\" AS \"DEPTNO0\", \"DEPT\".\"DNAME\", \"DEPT\".\"LOC\"\n"
9716+
+ "FROM \"scott\".\"EMP\"\n"
9717+
+ "INNER JOIN \"scott\".\"DEPT\" ON \"EMP\".\"DEPTNO\" = \"DEPT\".\"DEPTNO\"\n"
9718+
+ "FETCH NEXT 10 ROWS ONLY) AS \"t\"\n"
9719+
+ "INNER JOIN \"scott\".\"DEPT\" AS \"DEPT0\""
9720+
+ " ON \"t\".\"EMPNO\" = \"DEPT0\".\"DEPTNO\"\n"
9721+
+ "FETCH NEXT 5 ROWS ONLY";
9722+
relFn(relFn).withPostgresql().ok(expected);
9723+
}
9724+
96849725
/** Test case for
96859726
* <a href="https://issues.apache.org/jira/browse/CALCITE-7483">[CALCITE-7483]
96869727
* RelToSqlConverter generates SELECT * despite supportGenerateSelectStar</a>.

0 commit comments

Comments
 (0)