[CALCITE-1306] Allow GROUP BY and HAVING to reference SELECT expressions by ordinal and alias (Rajeshbabu Chintaguntla)
Close apache/calcite#413 Project: http://git-wip-us.apache.org/repos/asf/calcite/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/e046be23 Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/e046be23 Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/e046be23 Branch: refs/heads/master Commit: e046be23d7364e16648ade5240ad948a04cc814b Parents: 5ee895d Author: Rajeshbabu Chintaguntla <[email protected]> Authored: Thu Mar 30 08:25:19 2017 +0530 Committer: Julian Hyde <[email protected]> Committed: Tue May 9 16:24:25 2017 -0700 ---------------------------------------------------------------------- .../calcite/sql/validate/GroupByScope.java | 66 ++++++ .../sql/validate/SqlAbstractConformance.java | 12 ++ .../calcite/sql/validate/SqlConformance.java | 31 +++ .../sql/validate/SqlConformanceEnum.java | 27 +++ .../sql/validate/SqlDelegatingConformance.java | 13 ++ .../calcite/sql/validate/SqlValidatorImpl.java | 148 +++++++++++++- .../java/org/apache/calcite/test/JdbcTest.java | 2 +- .../calcite/test/SqlToRelConverterTest.java | 34 ++++ .../apache/calcite/test/SqlValidatorTest.java | 204 ++++++++++++++++++- .../calcite/test/SqlValidatorTestCase.java | 3 + .../calcite/test/SqlToRelConverterTest.xml | 77 +++++++ 11 files changed, 608 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite/blob/e046be23/core/src/main/java/org/apache/calcite/sql/validate/GroupByScope.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/sql/validate/GroupByScope.java b/core/src/main/java/org/apache/calcite/sql/validate/GroupByScope.java new file mode 100644 index 0000000..964fea9 --- /dev/null +++ b/core/src/main/java/org/apache/calcite/sql/validate/GroupByScope.java @@ -0,0 +1,66 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.calcite.sql.validate; + +import org.apache.calcite.sql.SqlNode; +import org.apache.calcite.sql.SqlNodeList; +import org.apache.calcite.sql.SqlSelect; + +/** + * Represents the name-resolution context for expressions in an GROUP BY clause. + * + * <p>In some dialects of SQL, the GROUP BY clause can reference column aliases + * in the SELECT clause. For example, the query</p> + * + * <blockquote><code>SELECT empno AS x<br> + * FROM emp<br> + * GROUP BY x</code></blockquote> + * + * <p>is valid.</p> + */ +public class GroupByScope extends DelegatingScope { + //~ Instance fields -------------------------------------------------------- + + private final SqlNodeList groupByList; + private final SqlSelect select; + + //~ Constructors ----------------------------------------------------------- + + GroupByScope( + SqlValidatorScope parent, + SqlNodeList groupByList, + SqlSelect select) { + super(parent); + this.groupByList = groupByList; + this.select = select; + } + + //~ Methods ---------------------------------------------------------------- + + public SqlNode getNode() { + return groupByList; + } + + public void validateExpr(SqlNode expr) { + SqlNode expanded = validator.expandGroupByOrHavingExpr(expr, this, select, false); + + // expression needs to be valid in parent scope too + parent.validateExpr(expanded); + } +} + +// End GroupByScope.java http://git-wip-us.apache.org/repos/asf/calcite/blob/e046be23/core/src/main/java/org/apache/calcite/sql/validate/SqlAbstractConformance.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/sql/validate/SqlAbstractConformance.java b/core/src/main/java/org/apache/calcite/sql/validate/SqlAbstractConformance.java index ddb4d7e..0c27dca 100644 --- a/core/src/main/java/org/apache/calcite/sql/validate/SqlAbstractConformance.java +++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlAbstractConformance.java @@ -23,6 +23,18 @@ package org.apache.calcite.sql.validate; * and behaves the same as in {@link SqlConformanceEnum#DEFAULT}. */ public abstract class SqlAbstractConformance implements SqlConformance { + public boolean isGroupByAlias() { + return SqlConformanceEnum.DEFAULT.isGroupByAlias(); + } + + public boolean isGroupByOrdinal() { + return SqlConformanceEnum.DEFAULT.isGroupByOrdinal(); + } + + public boolean isHavingAlias() { + return SqlConformanceEnum.DEFAULT.isHavingAlias(); + } + public boolean isSortByOrdinal() { return SqlConformanceEnum.DEFAULT.isSortByOrdinal(); } http://git-wip-us.apache.org/repos/asf/calcite/blob/e046be23/core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java b/core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java index 88af704..9d18268 100644 --- a/core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java +++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java @@ -62,6 +62,36 @@ public interface SqlConformance { SqlConformanceEnum PRAGMATIC_2003 = SqlConformanceEnum.PRAGMATIC_2003; /** + * Whether to allow aliases from the {@code SELECT} clause to be used as + * column names in the {@code GROUP BY} clause. + * + * <p>Among the built-in conformance levels, true in + * {@link SqlConformanceEnum#LENIENT}; + * false otherwise. + */ + boolean isGroupByAlias(); + + /** + * Whether {@code GROUP BY 2} is interpreted to mean 'group by the 2nd column + * in the select list'. + * + * <p>Among the built-in conformance levels, true in + * {@link SqlConformanceEnum#LENIENT}; + * false otherwise. + */ + boolean isGroupByOrdinal(); + + /** + * Whether to allow aliases from the {@code SELECT} clause to be used as + * column names in the {@code HAVING} clause. + * + * <p>Among the built-in conformance levels, true in + * {@link SqlConformanceEnum#LENIENT}; + * false otherwise. + */ + boolean isHavingAlias(); + + /** * Whether '{@code ORDER BY 2}' is interpreted to mean 'sort by the 2nd * column in the select list'. * @@ -208,6 +238,7 @@ public interface SqlConformance { * false otherwise. */ boolean allowNiladicParentheses(); + } // End SqlConformance.java http://git-wip-us.apache.org/repos/asf/calcite/blob/e046be23/core/src/main/java/org/apache/calcite/sql/validate/SqlConformanceEnum.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/sql/validate/SqlConformanceEnum.java b/core/src/main/java/org/apache/calcite/sql/validate/SqlConformanceEnum.java index 1ff0a10..226ec21 100644 --- a/core/src/main/java/org/apache/calcite/sql/validate/SqlConformanceEnum.java +++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlConformanceEnum.java @@ -62,6 +62,33 @@ public enum SqlConformanceEnum implements SqlConformance { * consistent with Microsoft SQL Server version 2008. */ SQL_SERVER_2008; + public boolean isGroupByAlias() { + switch (this) { + case LENIENT: + return true; + default: + return false; + } + } + + public boolean isGroupByOrdinal() { + switch (this) { + case LENIENT: + return true; + default: + return false; + } + } + + public boolean isHavingAlias() { + switch (this) { + case LENIENT: + return true; + default: + return false; + } + } + public boolean isSortByOrdinal() { switch (this) { case DEFAULT: http://git-wip-us.apache.org/repos/asf/calcite/blob/e046be23/core/src/main/java/org/apache/calcite/sql/validate/SqlDelegatingConformance.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/sql/validate/SqlDelegatingConformance.java b/core/src/main/java/org/apache/calcite/sql/validate/SqlDelegatingConformance.java index e1b4b4c..680cc60 100644 --- a/core/src/main/java/org/apache/calcite/sql/validate/SqlDelegatingConformance.java +++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlDelegatingConformance.java @@ -29,6 +29,18 @@ public class SqlDelegatingConformance extends SqlAbstractConformance { this.delegate = delegate; } + @Override public boolean isGroupByAlias() { + return delegate.isGroupByAlias(); + } + + @Override public boolean isGroupByOrdinal() { + return delegate.isGroupByOrdinal(); + } + + @Override public boolean isHavingAlias() { + return delegate.isGroupByAlias(); + } + @Override public boolean isSortByOrdinal() { return delegate.isSortByOrdinal(); } @@ -60,6 +72,7 @@ public class SqlDelegatingConformance extends SqlAbstractConformance { @Override public boolean allowNiladicParentheses() { return delegate.allowNiladicParentheses(); } + } // End SqlDelegatingConformance.java http://git-wip-us.apache.org/repos/asf/calcite/blob/e046be23/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java ---------------------------------------------------------------------- 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 28c4047..499cdc9 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 @@ -181,6 +181,12 @@ public class SqlValidatorImpl implements SqlValidatorWithHints { new IdentityHashMap<>(); /** + * Maps a {@link SqlSelect} node to the scope used by its GROUP BY clause. + */ + private final Map<SqlSelect, SqlValidatorScope> groupByScopes = + new IdentityHashMap<>(); + + /** * Maps a {@link SqlSelect} node to the scope used by its SELECT and HAVING * clauses. */ @@ -968,7 +974,7 @@ public class SqlValidatorImpl implements SqlValidatorWithHints { public SqlValidatorScope getGroupScope(SqlSelect select) { // Yes, it's the same as getWhereScope - return whereScopes.get(select); + return groupByScopes.get(select); } public SqlValidatorScope getFromScope(SqlSelect select) { @@ -2332,7 +2338,12 @@ public class SqlValidatorImpl implements SqlValidatorWithHints { } else { selectScopes.put(select, selectScope); } - registerSubQueries(selectScope, select.getGroup()); + if (select.getGroup() != null) { + GroupByScope groupByScope = + new GroupByScope(selectScope, select.getGroup(), select); + groupByScopes.put(select, groupByScope); + registerSubQueries(groupByScope, select.getGroup()); + } registerOperandSubQueries( aggScope, select, @@ -3472,6 +3483,17 @@ public class SqlValidatorImpl implements SqlValidatorWithHints { } /** + * Validates an item in the GROUP BY clause of a SELECT statement. + * + * @param select Select statement + * @param groupByItem GROUP BY clause item + */ + private void validateGroupByItem(SqlSelect select, SqlNode groupByItem) { + final SqlValidatorScope groupByScope = getGroupScope(select); + groupByScope.validateExpr(groupByItem); + } + + /** * Validates an item in the ORDER BY clause of a SELECT statement. * * @param select Select statement @@ -3519,11 +3541,14 @@ public class SqlValidatorImpl implements SqlValidatorWithHints { // expand the expression in group list. List<SqlNode> expandedList = new ArrayList<>(); for (SqlNode groupItem : groupList) { - SqlNode expandedItem = expand(groupItem, groupScope); + SqlNode expandedItem = expandGroupByOrHavingExpr(groupItem, groupScope, select, false); expandedList.add(expandedItem); } groupList = new SqlNodeList(expandedList, groupList.getParserPosition()); select.setGroupBy(groupList); + for (SqlNode groupItem : expandedList) { + validateGroupByItem(select, groupItem); + } // Nodes in the GROUP BY clause are expressions except if they are calls // to the GROUPING SETS, ROLLUP or CUBE operators; this operators are not @@ -3620,12 +3645,19 @@ public class SqlValidatorImpl implements SqlValidatorWithHints { // For example, in "SELECT empno FROM emp WHERE empno = 10 GROUP BY // deptno HAVING empno = 10", the reference to 'empno' in the HAVING // clause is illegal. - final SqlNode having = select.getHaving(); + SqlNode having = select.getHaving(); if (having == null) { return; } final AggregatingScope havingScope = (AggregatingScope) getSelectScope(select); + if (getConformance().isHavingAlias()) { + SqlNode newExpr = expandGroupByOrHavingExpr(having, havingScope, select, true); + if (having != newExpr) { + having = newExpr; + select.setHaving(newExpr); + } + } havingScope.checkAggregateExpr(having, true); inferUnknownTypes( booleanType, @@ -4699,6 +4731,16 @@ public class SqlValidatorImpl implements SqlValidatorWithHints { return newExpr; } + public SqlNode expandGroupByOrHavingExpr(SqlNode expr, SqlValidatorScope scope, SqlSelect select, + boolean havingExpression) { + final Expander expander = new ExtendedExpander(this, scope, select, expr, havingExpression); + SqlNode newExpr = expr.accept(expander); + if (expr != newExpr) { + setOriginal(newExpr, expr); + } + return newExpr; + } + public boolean isSystemField(RelDataTypeField field) { return false; } @@ -5069,7 +5111,7 @@ public class SqlValidatorImpl implements SqlValidatorWithHints { * identifiers. */ private static class Expander extends SqlScopedShuttle { - private final SqlValidatorImpl validator; + protected final SqlValidatorImpl validator; Expander(SqlValidatorImpl validator, SqlValidatorScope scope) { super(scope); @@ -5229,6 +5271,102 @@ public class SqlValidatorImpl implements SqlValidatorWithHints { } } + /** + * Shuttle which walks over an expression in the GROUP BY/HAVING clause, replacing + * usages of aliases or ordinals with the underlying expression. + */ + static class ExtendedExpander extends Expander { + final SqlSelect select; + final SqlNode root; + final boolean havingExpr; + + ExtendedExpander(SqlValidatorImpl validator, SqlValidatorScope scope, + SqlSelect select, SqlNode root, boolean havingExpr) { + super(validator, scope); + this.select = select; + this.root = root; + this.havingExpr = havingExpr; + } + + @Override public SqlNode visit(SqlIdentifier id) { + if (id.isSimple() + && (havingExpr + ? validator.getConformance().isHavingAlias() + : validator.getConformance().isGroupByAlias())) { + String name = id.getSimple(); + SqlNode expr = null; + final SqlNameMatcher nameMatcher = + validator.catalogReader.nameMatcher(); + int n = 0; + for (SqlNode s : select.getSelectList()) { + final String alias = SqlValidatorUtil.getAlias(s, -1); + if (alias != null && nameMatcher.matches(alias, name)) { + expr = s; + n++; + } + } + if (n == 0) { + return super.visit(id); + } else if (n > 1) { + // More than one column has this alias. + throw validator.newValidationError(id, + RESOURCE.columnAmbiguous(name)); + } + if (havingExpr && validator.isAggregate(root)) { + return super.visit(id); + } + expr = stripAs(expr); + if (expr instanceof SqlIdentifier) { + expr = getScope().fullyQualify((SqlIdentifier) expr).identifier; + } + return expr; + } + return super.visit(id); + } + + public SqlNode visit(SqlLiteral literal) { + if (havingExpr || !validator.getConformance().isGroupByOrdinal()) { + return super.visit(literal); + } + boolean isOrdinalLiteral = literal == root; + switch (root.getKind()) { + case GROUPING_SETS: + case ROLLUP: + case CUBE: + if (root instanceof SqlBasicCall) { + List<SqlNode> operandList = ((SqlBasicCall) root).getOperandList(); + for (SqlNode node : operandList) { + if (node.equals(literal)) { + isOrdinalLiteral = true; + break; + } + } + } + break; + } + if (isOrdinalLiteral) { + switch (literal.getTypeName()) { + case DECIMAL: + case DOUBLE: + final int intValue = literal.intValue(false); + if (intValue >= 0) { + if (intValue < 1 || intValue > select.getSelectList().size()) { + throw validator.newValidationError(literal, + RESOURCE.orderByOrdinalOutOfRange()); + } + + // SQL ordinals are 1-based, but Sort's are 0-based + int ordinal = intValue - 1; + return select.getSelectList().get(ordinal); + } + break; + } + } + + return super.visit(literal); + } + } + /** Information about an identifier in a particular scope. */ protected static class IdInfo { public final SqlValidatorScope scope; http://git-wip-us.apache.org/repos/asf/calcite/blob/e046be23/core/src/test/java/org/apache/calcite/test/JdbcTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/test/JdbcTest.java b/core/src/test/java/org/apache/calcite/test/JdbcTest.java index 62a526b..fb576dc 100644 --- a/core/src/test/java/org/apache/calcite/test/JdbcTest.java +++ b/core/src/test/java/org/apache/calcite/test/JdbcTest.java @@ -6168,7 +6168,7 @@ public class JdbcTest { /** Tests case-insensitive resolution of sub-query columns. * * <p>Test case for - * <a href="https://issues.apache.org/jira/browse/CALCITE-555";>[CALCITE-555] + * <a href="https://issues.apache.org/jira/browse/CALCITE-550";>[CALCITE-550] * Case-insensitive matching of sub-query columns fails</a>. */ @Test public void testLexCaseInsensitiveSubQueryField() { CalciteAssert.that() http://git-wip-us.apache.org/repos/asf/calcite/blob/e046be23/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java ---------------------------------------------------------------------- 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 01b1897..4bdd791 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java @@ -237,6 +237,40 @@ public class SqlToRelConverterTest extends SqlToRelTestBase { sql("select deptno from emp group by deptno").ok(); } + @Test public void testGroupByAlias() { + sql("select empno as d from emp group by d") + .conformance(SqlConformanceEnum.LENIENT).ok(); + } + + @Test public void testGroupByAliasOfSubExpressionsInProject() { + final String sql = "select deptno+empno as d, deptno+empno+mgr\n" + + "from emp group by d,mgr"; + sql(sql) + .conformance(SqlConformanceEnum.LENIENT).ok(); + } + + @Test public void testGroupByAliasEqualToColumnName() { + sql("select empno, ename as deptno from emp group by empno, deptno") + .conformance(SqlConformanceEnum.LENIENT).ok(); + } + + @Test public void testGroupByOrdinal() { + sql("select empno from emp group by 1") + .conformance(SqlConformanceEnum.LENIENT).ok(); + } + + @Test public void testGroupByContainsLiterals() { + final String sql = "select count(*) from (\n" + + " select 1 from emp group by substring(ename from 2 for 3))"; + sql(sql) + .conformance(SqlConformanceEnum.LENIENT).ok(); + } + + @Test public void testAliasInHaving() { + sql("select count(empno) as e from emp having e > 1") + .conformance(SqlConformanceEnum.LENIENT).ok(); + } + @Test public void testGroupJustOneAgg() { // just one agg final String sql = http://git-wip-us.apache.org/repos/asf/calcite/blob/e046be23/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java ---------------------------------------------------------------------- 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 ae1e1f8..c902669 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java @@ -6116,6 +6116,197 @@ public class SqlValidatorTest extends SqlValidatorTestCase { } /** + * Tests validation of the aliases in GROUP BY. + * + * <p>Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-1306";>[CALCITE-1306] + * Allow GROUP BY and HAVING to reference SELECT expressions by ordinal and + * alias</a>. + * + * @see SqlConformance#isGroupByAlias() + */ + @Test public void testAliasInGroupBy() { + final SqlTester lenient = + tester.withConformance(SqlConformanceEnum.LENIENT); + final SqlTester strict = + tester.withConformance(SqlConformanceEnum.STRICT_2003); + + // Group by + sql("select empno as e from emp group by ^e^") + .tester(strict).fails("Column 'E' not found in any table") + .tester(lenient).sansCarets().ok(); + sql("select empno as e from emp group by ^e^") + .tester(strict).fails("Column 'E' not found in any table") + .tester(lenient).sansCarets().ok(); + sql("select emp.empno as e from emp group by ^e^") + .tester(strict).fails("Column 'E' not found in any table") + .tester(lenient).sansCarets().ok(); + sql("select e.empno from emp as e group by e.empno") + .tester(strict).ok() + .tester(lenient).ok(); + sql("select e.empno as eno from emp as e group by ^eno^") + .tester(strict).fails("Column 'ENO' not found in any table") + .tester(lenient).sansCarets().ok(); + sql("select deptno as dno from emp group by cube(^dno^)") + .tester(strict).fails("Column 'DNO' not found in any table") + .tester(lenient).sansCarets().ok(); + sql("select deptno as dno, ename name, sum(sal) from emp\n" + + "group by grouping sets ((^dno^), (name, deptno))") + .tester(strict).fails("Column 'DNO' not found in any table") + .tester(lenient).sansCarets().ok(); + sql("select ename as deptno from emp as e join dept as d on " + + "e.deptno = d.deptno group by ^deptno^") + .tester(lenient).sansCarets().ok(); + sql("select t.e, count(*) from (select empno as e from emp) t group by e") + .tester(strict).ok() + .tester(lenient).ok(); + + // The following 2 tests have the same SQL but fail for different reasons. + sql("select t.e, count(*) as c from " + + " (select empno as e from emp) t group by e,^c^") + .tester(strict).fails("Column 'C' not found in any table"); + sql("select t.e, ^count(*)^ as c from " + + " (select empno as e from emp) t group by e,c") + .tester(lenient).fails(ERR_AGG_IN_GROUP_BY); + + sql("select t.e, e + ^count(*)^ as c from " + + " (select empno as e from emp) t group by e,c") + .tester(lenient).fails(ERR_AGG_IN_GROUP_BY); + sql("select t.e, e + ^count(*)^ as c from " + + " (select empno as e from emp) t group by e,2") + .tester(lenient).fails(ERR_AGG_IN_GROUP_BY) + .tester(strict).sansCarets().ok(); + + sql("select deptno,(select empno + 1 from emp) eno\n" + + "from dept group by deptno,^eno^") + .tester(strict).fails("Column 'ENO' not found in any table") + .tester(lenient).sansCarets().ok(); + sql("select empno as e, deptno as e\n" + + "from emp group by ^e^") + .tester(lenient).fails("Column 'E' is ambiguous"); + sql("select empno, ^count(*)^ c from emp group by empno, c") + .tester(lenient).fails(ERR_AGG_IN_GROUP_BY); + sql("select deptno + empno as d, deptno + empno + mgr from emp" + + " group by d,mgr") + .tester(lenient).sansCarets().ok(); + // When alias is equal to one or more columns in the query then giving + // priority to alias. But Postgres may throw ambiguous column error or give + // priority to column name. + sql("select count(*) from (\n" + + " select ename AS deptno FROM emp GROUP BY deptno) t") + .tester(lenient).sansCarets().ok(); + sql("select count(*) from " + + "(select ename AS deptno FROM emp, dept GROUP BY deptno) t") + .tester(lenient).sansCarets().ok(); + sql("select empno + deptno AS \"z\" FROM emp GROUP BY \"Z\"") + .tester(lenient.withCaseSensitive(false)).sansCarets().ok(); + sql("select empno + deptno as c, ^c^ + mgr as d from emp group by c, d") + .tester(lenient).fails("Column 'C' not found in any table"); + // Group by alias with strict conformance should fail. + sql("select empno as e from emp group by ^e^") + .tester(strict).fails("Column 'E' not found in any table"); + } + + /** + * Tests validation of ordinals in GROUP BY. + * + * @see SqlConformance#isGroupByOrdinal() + */ + @Test public void testOrdinalInGroupBy() { + final SqlTester lenient = + tester.withConformance(SqlConformanceEnum.LENIENT); + final SqlTester strict = + tester.withConformance(SqlConformanceEnum.STRICT_2003); + + sql("select ^empno^,deptno from emp group by 1, deptno") + .tester(strict).fails("Expression 'EMPNO' is not being grouped") + .tester(lenient).sansCarets().ok(); + sql("select ^emp.empno^ as e from emp group by 1") + .tester(strict).fails("Expression 'EMP.EMPNO' is not being grouped") + .tester(lenient).sansCarets().ok(); + sql("select 2 + ^emp.empno^ + 3 as e from emp group by 1") + .tester(strict).fails("Expression 'EMP.EMPNO' is not being grouped") + .tester(lenient).sansCarets().ok(); + sql("select ^e.empno^ from emp as e group by 1") + .tester(strict).fails("Expression 'E.EMPNO' is not being grouped") + .tester(lenient).sansCarets().ok(); + sql("select e.empno from emp as e group by 1, empno") + .tester(strict).ok() + .tester(lenient).sansCarets().ok(); + sql("select ^e.empno^ as eno from emp as e group by 1") + .tester(strict).fails("Expression 'E.EMPNO' is not being grouped") + .tester(lenient).sansCarets().ok(); + sql("select ^deptno^ as dno from emp group by cube(1)") + .tester(strict).fails("Expression 'DEPTNO' is not being grouped") + .tester(lenient).sansCarets().ok(); + sql("select 1 as dno from emp group by cube(1)") + .tester(strict).ok() + .tester(lenient).sansCarets().ok(); + sql("select deptno as dno, ename name, sum(sal) from emp\n" + + "group by grouping sets ((1), (^name^, deptno))") + .tester(strict).fails("Column 'NAME' not found in any table") + .tester(lenient).sansCarets().ok(); + sql("select ^e.deptno^ from emp as e\n" + + "join dept as d on e.deptno = d.deptno group by 1") + .tester(strict).fails("Expression 'E.DEPTNO' is not being grouped") + .tester(lenient).sansCarets().ok(); + sql("select ^deptno^,(select empno from emp) eno from dept" + + " group by 1,2") + .tester(strict).fails("Expression 'DEPTNO' is not being grouped") + .tester(lenient).sansCarets().ok(); + sql("select count(*) from (select 1 from emp" + + " group by substring(ename from 2 for 3))") + .tester(strict).ok() + .tester(lenient).sansCarets().ok(); + sql("select deptno from emp group by deptno, ^100^") + .tester(lenient).fails("Ordinal out of range") + .tester(strict).sansCarets().ok(); + // Calcite considers integers in GROUP BY to be constants, so test passes. + // Postgres considers them ordinals and throws out of range position error. + sql("select deptno from emp group by ^100^, deptno") + .tester(lenient).fails("Ordinal out of range") + .tester(strict).sansCarets().ok(); + } + + /** + * Tests validation of the aliases in HAVING. + * + * @see SqlConformance#isHavingAlias() + */ + @Test public void testAliasInHaving() { + final SqlTester lenient = + tester.withConformance(SqlConformanceEnum.LENIENT); + final SqlTester strict = + tester.withConformance(SqlConformanceEnum.STRICT_2003); + + sql("select count(empno) as e from emp having ^e^ > 10") + .tester(strict).fails("Column 'E' not found in any table") + .tester(lenient).sansCarets().ok(); + sql("select emp.empno as e from emp group by ^e^ having e > 10") + .tester(strict).fails("Column 'E' not found in any table") + .tester(lenient).sansCarets().ok(); + sql("select emp.empno as e from emp group by empno having ^e^ > 10") + .tester(strict).fails("Column 'E' not found in any table") + .tester(lenient).sansCarets().ok(); + sql("select e.empno from emp as e group by 1 having ^e.empno^ > 10") + .tester(strict).fails("Expression 'E.EMPNO' is not being grouped") + .tester(lenient).sansCarets().ok(); + // When alias is equal to one or more columns in the query then giving + // priority to alias, but PostgreSQL throws ambiguous column error or gives + // priority to column name. + sql("select count(empno) as deptno from emp having ^deptno^ > 10") + .tester(strict).fails("Expression 'DEPTNO' is not being grouped") + .tester(lenient).sansCarets().ok(); + // Alias in aggregate is not allowed. + sql("select empno as e from emp having max(^e^) > 10") + .tester(strict).fails("Column 'E' not found in any table") + .tester(lenient).fails("Column 'E' not found in any table"); + sql("select count(empno) as e from emp having ^e^ > 10") + .tester(strict).fails("Column 'E' not found in any table") + .tester(lenient).sansCarets().ok(); + } + + /** * Tests validation of the ORDER BY clause when DISTINCT is present. */ @Test public void testOrderDistinct() { @@ -7247,9 +7438,16 @@ public class SqlValidatorTest extends SqlValidatorTestCase { check("SELECT deptno FROM emp GROUP BY deptno HAVING deptno > 55"); check("SELECT DISTINCT deptno, 33 FROM emp\n" + "GROUP BY deptno HAVING deptno > 55"); - checkFails( - "SELECT DISTINCT deptno, 33 FROM emp HAVING ^deptno^ > 55", - "Expression 'DEPTNO' is not being grouped"); + sql("SELECT DISTINCT deptno, 33 FROM emp HAVING ^deptno^ > 55") + .fails("Expression 'DEPTNO' is not being grouped"); + // same query under a different conformance finds a different error first + sql("SELECT DISTINCT ^deptno^, 33 FROM emp HAVING deptno > 55") + .tester(tester.withConformance(SqlConformanceEnum.LENIENT)) + .fails("Expression 'DEPTNO' is not being grouped"); + sql("SELECT DISTINCT 33 FROM emp HAVING ^deptno^ > 55") + .fails("Expression 'DEPTNO' is not being grouped") + .tester(tester.withConformance(SqlConformanceEnum.LENIENT)) + .fails("Expression 'DEPTNO' is not being grouped"); check("SELECT DISTINCT * from emp"); checkFails( "SELECT DISTINCT ^*^ from emp GROUP BY deptno", http://git-wip-us.apache.org/repos/asf/calcite/blob/e046be23/core/src/test/java/org/apache/calcite/test/SqlValidatorTestCase.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTestCase.java b/core/src/test/java/org/apache/calcite/test/SqlValidatorTestCase.java index cc66e3f..b5db2c3 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTestCase.java +++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTestCase.java @@ -581,6 +581,9 @@ public class SqlValidatorTestCase { return this; } + /** Removes the carets from the SQL string. Useful if you want to run + * a test once at a conformance level where it fails, then run it again + * at a conformance level where it succeeds. */ public Sql sansCarets() { return new Sql(tester, sql.replace("^", "")); } http://git-wip-us.apache.org/repos/asf/calcite/blob/e046be23/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml ---------------------------------------------------------------------- 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 13257e2..4cb6f9d 100644 --- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml @@ -39,6 +39,83 @@ LogicalAggregate(group=[{0}]) <![CDATA[select deptno from emp group by deptno]]> </Resource> </TestCase> + <TestCase name="testGroupByAlias"> + <Resource name="plan"> + <![CDATA[ +LogicalAggregate(group=[{0}]) + LogicalProject(D=[$0]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + </Resource> + <Resource name="sql"> + <![CDATA[select empno as d from emp group by d]]> + </Resource> + </TestCase> + <TestCase name="testGroupByAliasEqualToColumnName"> + <Resource name="plan"> + <![CDATA[ +LogicalAggregate(group=[{0, 1}]) + LogicalProject(EMPNO=[$0], DEPTNO=[$1]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + </Resource> + <Resource name="sql"> + <![CDATA[select empno, ename as deptno from emp group by empno, deptno]]> + </Resource> + </TestCase> + <TestCase name="testGroupByAliasOfSubExpressionsInProject"> + <Resource name="plan"> + <![CDATA[ +LogicalProject(D=[$0], EXPR$1=[+($0, $1)]) + LogicalAggregate(group=[{0, 1}]) + LogicalProject(D=[+($7, $0)], MGR=[$3]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + </Resource> + <Resource name="sql"> + <![CDATA[select deptno+empno as d, deptno+empno+mgr from emp group by d,mgr]]> + </Resource> + </TestCase> + <TestCase name="testGroupByOrdinal"> + <Resource name="plan"> + <![CDATA[ +LogicalAggregate(group=[{0}]) + LogicalProject(EMPNO=[$0]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + </Resource> + <Resource name="sql"> + <![CDATA[select empno from emp group by 1]]> + </Resource> + </TestCase> + <TestCase name="testGroupByContainsLiterals"> + <Resource name="plan"> + <![CDATA[ +LogicalAggregate(group=[{}], EXPR$0=[COUNT()]) + LogicalProject($f0=[0]) + LogicalProject(EXPR$0=[1]) + LogicalAggregate(group=[{0}]) + LogicalProject($f0=[SUBSTRING($1, 2, 3)]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + </Resource> + <Resource name="sql"> + <![CDATA[select count(*) from (select 1 from emp group by substring(ename from 2 for 3))]]> + </Resource> + </TestCase> + <TestCase name="testAliasInHaving"> + <Resource name="plan"> + <![CDATA[ +LogicalFilter(condition=[>($0, 1)]) + LogicalAggregate(group=[{}], E=[COUNT()]) + LogicalProject(EMPNO=[$0]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + </Resource> + <Resource name="sql"> + <![CDATA[select count(empno) as e from emp having e > 1]]> + </Resource> + </TestCase> <TestCase name="testGroupJustOneAgg"> <Resource name="plan"> <