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 e21d62aecbf..1b22d586def 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 @@ -643,6 +643,31 @@ private static void validateQualifiedCommonColumn(SqlJoin join, } } + /** Validates that a SQL node tree does not contain qualified references + * to common columns in a JOIN USING or NATURAL JOIN context. + * This is called before identifier expansion to catch user-written + * qualified common columns in conformances where they are disallowed + * (e.g. Oracle, Presto). + * + * @param nodeList The list of SQL nodes to check + * @param join The JOIN node containing USING/NATURAL condition + * @param scope The select scope for resolving identifiers + */ + private void validateNoQualifiedCommonColumns(SqlNodeList nodeList, + SqlJoin join, SelectScope scope) { + for (SqlNode item : nodeList) { + item.accept(new SqlShuttle() { + @Override public SqlNode visit(SqlIdentifier id) { + if (!id.isSimple()) { + validateQualifiedCommonColumn(join, id, scope, + SqlValidatorImpl.this); + } + return id; + } + }); + } + } + private boolean expandStar(List selectItems, Set aliases, PairList fields, boolean includeSystemVars, SelectScope scope, SqlNode node) { @@ -5253,6 +5278,17 @@ protected void validateGroupClause(SqlSelect select) { // expand the expression in group list. List expandedList = new ArrayList<>(); + // Validate that GROUP BY items do not qualify common columns + // in conformances where it is disallowed (e.g. Oracle, Presto). + // This must run before expansion, because expansion generates + // qualified identifiers that should not trigger this validation. + if (!config.conformance().allowQualifyingCommonColumn()) { + final SqlNode from = select.getFrom(); + if (from instanceof SqlJoin) { + validateNoQualifiedCommonColumns(groupList, + (SqlJoin) from, getRawSelectScopeNonNull(select)); + } + } for (SqlNode groupItem : groupList) { SqlNode expandedItem = extendedExpand(groupItem, groupScope, select, Clause.GROUP_BY); @@ -5404,6 +5440,19 @@ protected void validateHavingClause(SqlSelect select) { } } + /** Validates that SELECT items do not qualify common columns + * in conformances where it is disallowed (e.g. Oracle, Presto). */ + private void validateSelectCommonColumns(SqlNodeList selectItems, + SqlSelect select) { + if (!config().conformance().allowQualifyingCommonColumn()) { + final SqlNode from = select.getFrom(); + if (from instanceof SqlJoin) { + validateNoQualifiedCommonColumns(selectItems, + (SqlJoin) from, getRawSelectScopeNonNull(select)); + } + } + } + protected RelDataType validateSelectList(final SqlNodeList selectItems, SqlSelect select, RelDataType targetRowType) { // First pass, ensure that aliases are unique. "*" and "TABLE.*" items @@ -5417,6 +5466,12 @@ protected RelDataType validateSelectList(final SqlNodeList selectItems, // Populated during select expansion when SqlConformance.isSelectAlias != UNSUPPORTED final Map expansions = new HashMap<>(); + // Validate that SELECT items do not qualify common columns + // in conformances where it is disallowed (e.g. Oracle, Presto). + // This must run before expansion, because expansion generates + // qualified identifiers that should not trigger this validation. + validateSelectCommonColumns(selectItems, select); + for (SqlNode selectItem : selectItems) { if (selectItem instanceof SqlSelect) { handleScalarSubQuery(select, (SqlSelect) selectItem, @@ -7672,9 +7727,10 @@ protected SqlNode expandCommonColumn(SqlSelect sqlSelect, SqlNode selectItem, final SqlIdentifier identifier = (SqlIdentifier) selectItem; if (!identifier.isSimple()) { - if (!validator.config().conformance().allowQualifyingCommonColumn()) { - validateQualifiedCommonColumn((SqlJoin) from, identifier, scope, validator); - } + // Qualified identifiers (e.g. t1.col) are returned unchanged. + // Validation of qualified common columns is performed before expansion, + // in validateSelectCommonColumns, where the original user-written + // identifier (with its source position) is still available. return selectItem; } @@ -7700,10 +7756,14 @@ private SqlNode expandExprFromJoin(SqlJoin join, SqlIdentifier identifier, Selec final List qualifiedNode = new ArrayList<>(); for (ScopeChild child : requireNonNull(scope, "scope").children) { if (matcher.indexOf(child.namespace.getRowType().getFieldNames(), name) >= 0) { + // These identifiers are created by the expansion logic, not + // parsed from user input, so they have no meaningful source + // position. SqlParserPos.ZERO is the standard convention for + // internally generated nodes. final SqlIdentifier exp = new SqlIdentifier( ImmutableList.of(child.name, name), - identifier.getParserPosition()); + SqlParserPos.ZERO); qualifiedNode.add(exp); } } diff --git a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java index 4bc62e7c3d5..ebad0e9450a 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java @@ -6092,4 +6092,45 @@ void checkUserDefinedOrderByOver(NullCollation nullCollation) { final String sql = "select distinct deptno, deptno, empno, 1, 'a' from emp order by rand(), 1"; sql(sql).ok(); } + + /** Test case of + * [CALCITE-7085] + * JOIN USING with unqualified common column fails in a conformance where + * allowQualifyingCommonColumn is false (e.g. Oracle, Presto). */ + @Test void testJoinUsingWithConformanceOracle() { + final String sql = "SELECT deptno, name\n" + + "FROM emp JOIN dept using (deptno)"; + sql(sql).withConformance(SqlConformanceEnum.ORACLE_10).ok(); + } + + + /** Test case of + * [CALCITE-7085] + * JOIN USING with unqualified common column fails in a conformance where + * allowQualifyingCommonColumn is false (e.g. Oracle, Presto). */ + @Test void testLeftJoinUsingWithConformanceOracle() { + final String sql = "SELECT deptno, name\n" + + "FROM emp LEFT OUTER JOIN dept using (deptno)"; + sql(sql).withConformance(SqlConformanceEnum.ORACLE_10).ok(); + } + + /** Test case of + * [CALCITE-7085] + * JOIN USING with unqualified common column fails in a conformance where + * allowQualifyingCommonColumn is false (e.g. Oracle, Presto). */ + @Test void testRightJoinUsingWithConformanceOracle() { + final String sql = "SELECT deptno, name\n" + + "FROM emp RIGHT OUTER JOIN dept using (deptno)"; + sql(sql).withConformance(SqlConformanceEnum.ORACLE_10).ok(); + } + + /** Test case of + * [CALCITE-7085] + * JOIN USING with unqualified common column fails in a conformance where + * allowQualifyingCommonColumn is false (e.g. Oracle, Presto). */ + @Test void testJoinUsingWithConformancePresto() { + final String sql = "SELECT deptno, name\n" + + "FROM emp JOIN dept using (deptno)"; + sql(sql).withConformance(SqlConformanceEnum.PRESTO).ok(); + } } 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 53662875e0d..56f169629ae 100644 --- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml @@ -4368,6 +4368,34 @@ LogicalProject(DEPTNO=[$0], EXPR$1=[$1]) LogicalJoin(condition=[=($7, $9)], joinType=[full]) LogicalTableScan(table=[[CATALOG, SALES, EMP]]) LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) +]]> + + + + + + + + + + + + + + + + @@ -5000,6 +5028,20 @@ LogicalProject(C=[$0], N=[$3]) LogicalAggregate(group=[{0}]) LogicalProject($f2=[+($0, 1)]) LogicalValues(tuples=[[{ 4 }]]) +]]> + + + + + + + + @@ -7396,6 +7438,20 @@ GROUP BY ROLLUP(deptno)]]> LogicalAggregate(group=[{0}], groups=[[{0}, {}]], EXPR$1=[COUNT(DISTINCT $1)]) LogicalProject(DEPTNO=[$7], EMPNO=[$0]) LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + + + + + + + +