This is an automated email from the ASF dual-hosted git repository. jhyde pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 2d2839458d20b083762b5ffffef1d0b9dba4ef73 Author: xiejiajun <[email protected]> AuthorDate: Mon Jan 30 20:37:53 2023 +0800 [CALCITE-5507] HAVING alias fails for mixed usage of alias and aggregate function Close apache/calcite#3055 --- .../calcite/sql/validate/SqlValidatorImpl.java | 32 ++++++++++++++++++++-- .../apache/calcite/test/SqlToRelConverterTest.java | 17 ++++++++++++ .../org/apache/calcite/test/SqlValidatorTest.java | 15 ++++++++++ .../apache/calcite/test/SqlToRelConverterTest.xml | 30 ++++++++++++++++++++ 4 files changed, 92 insertions(+), 2 deletions(-) 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 99e52c5e4e..bb27150de4 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 @@ -6822,8 +6822,11 @@ public class SqlValidatorImpl implements SqlValidatorWithHints { throw validator.newValidationError(id, RESOURCE.columnAmbiguous(name)); } - if (havingExpr && validator.isAggregate(root)) { - return super.visit(id); + Iterable<SqlCall> allAggList = validator.aggFinder.findAll(ImmutableList.of(root)); + for (SqlCall agg : allAggList) { + if (havingExpr && containsIdentifier(agg, id)) { + return super.visit(id); + } } expr = stripAs(expr); if (expr instanceof SqlIdentifier) { @@ -6888,6 +6891,31 @@ public class SqlValidatorImpl implements SqlValidatorWithHints { return super.visit(literal); } + + /** + * Returns whether a given node contains a {@link SqlIdentifier}. + * + * @param sqlNode a SqlNode + * @param target a SqlIdentifier + */ + private boolean containsIdentifier(SqlNode sqlNode, SqlIdentifier target) { + try { + SqlVisitor<Void> visitor = + new SqlBasicVisitor<Void>() { + @Override public Void visit(SqlIdentifier identifier) { + if (identifier.equalsDeep(target, Litmus.IGNORE)) { + throw new Util.FoundOne(target); + } + return super.visit(identifier); + } + }; + sqlNode.accept(visitor); + return false; + } catch (Util.FoundOne e) { + Util.swallow(e, null); + return true; + } + } } /** Information about an identifier in a particular scope. */ 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 8035778490..165c24d194 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java @@ -385,6 +385,23 @@ class SqlToRelConverterTest extends SqlToRelTestBase { .withConformance(SqlConformanceEnum.LENIENT).ok(); } + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-5507";>[CALCITE-5507] + * HAVING alias failed when aggregate function in condition</a>. */ + @Test void testAggregateFunAndAliasInHaving1() { + sql("select count(empno) as e\n" + + "from emp\n" + + "having e > 10 and count(empno) > 10") + .withConformance(SqlConformanceEnum.LENIENT).ok(); + } + + @Test void testAggregateFunAndAliasInHaving2() { + sql("select count(empno) as e\n" + + "from emp\n" + + "having e > 10 or count(empno) < 5") + .withConformance(SqlConformanceEnum.LENIENT).ok(); + } + @Test void testGroupJustOneAgg() { // just one agg final String sql = 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 17cc9306af..f118ab3bc3 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java @@ -5673,6 +5673,21 @@ public class SqlValidatorTest extends SqlValidatorTestCase { .withConformance(strict).ok(); } + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-5507";>[CALCITE-5507] + * HAVING alias failed when aggregate function in condition</a>. */ + @Test void testAggregateFunAndAliasInHaving() { + final SqlConformanceEnum lenient = SqlConformanceEnum.LENIENT; + final SqlConformanceEnum strict = SqlConformanceEnum.STRICT_2003; + + sql("select count(empno) as e from emp having ^e^ > 10 and count(empno) > 10 ") + .withConformance(strict).fails("Column 'E' not found in any table") + .withConformance(lenient).ok(); + sql("select count(empno) as e from emp having count(empno) > 10 and count(^e^) > 10") + .withConformance(strict).fails("Column 'E' not found in any table") + .withConformance(lenient).fails("Column 'E' not found in any table"); + } + /** * Tests validation of the aliases in HAVING. * 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 4630dc4ba4..30620409f1 100644 --- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml @@ -161,6 +161,36 @@ LogicalProject(DEPTNO=[$0], B=[>($1, $2)]) LogicalAggregate(group=[{}], EXPR$0=[MIN($0)]) LogicalProject(EMPNO=[$0]) LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + </Resource> + </TestCase> + <TestCase name="testAggregateFunAndAliasInHaving1"> + <Resource name="sql"> + <![CDATA[select count(empno) as e +from emp +having e > 10 and count(empno) > 10]]> + </Resource> + <Resource name="plan"> + <![CDATA[ +LogicalFilter(condition=[>($0, 10)]) + LogicalAggregate(group=[{}], E=[COUNT()]) + LogicalProject(EMPNO=[$0]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) +]]> + </Resource> + </TestCase> + <TestCase name="testAggregateFunAndAliasInHaving2"> + <Resource name="sql"> + <![CDATA[select count(empno) as e +from emp +having e > 10 or count(empno) < 5]]> + </Resource> + <Resource name="plan"> + <![CDATA[ +LogicalFilter(condition=[SEARCH($0, Sarg[(-∞..5), (10..+∞)])]) + LogicalAggregate(group=[{}], E=[COUNT()]) + LogicalProject(EMPNO=[$0]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) ]]> </Resource> </TestCase>
