This is an automated email from the ASF dual-hosted git repository.
mbudiu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new 1294b4e8cc [CALCITE-4512] GROUP BY expression with argument name same
with SELECT field and alias causes validation error
1294b4e8cc is described below
commit 1294b4e8cc3dcee8db8dd73ad93315299d561067
Author: wangyanjing <[email protected]>
AuthorDate: Thu Aug 22 19:49:01 2024 +0800
[CALCITE-4512] GROUP BY expression with argument name same with SELECT
field and alias causes validation error
---
.../calcite/sql/validate/SqlValidatorImpl.java | 71 ++++++++++++++++------
.../apache/calcite/test/SqlToRelConverterTest.java | 31 ++++++++++
.../org/apache/calcite/test/SqlValidatorTest.java | 40 ++++++++++++
.../apache/calcite/test/SqlToRelConverterTest.xml | 39 ++++++++++++
core/src/test/resources/sql/misc.iq | 18 ++++++
5 files changed, 180 insertions(+), 19 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 af109c47ea..ba4e3203a9 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
@@ -127,10 +127,12 @@ import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import org.apiguardian.api.API;
+import org.checkerframework.checker.initialization.qual.UnknownInitialization;
import org.checkerframework.checker.nullness.qual.KeyFor;
import org.checkerframework.checker.nullness.qual.NonNull;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.checkerframework.checker.nullness.qual.PolyNull;
+import org.checkerframework.checker.nullness.qual.RequiresNonNull;
import org.checkerframework.dataflow.qual.Pure;
import org.slf4j.Logger;
@@ -7302,6 +7304,8 @@ public class SqlValidatorImpl implements
SqlValidatorWithHints {
final SqlSelect select;
final SqlNode root;
final Clause clause;
+ // Retain only expandable aliases or ordinals to prevent their expansion
in a SQL call expr.
+ final Set<SqlNode> aliasOrdinalExpandSet = Sets.newIdentityHashSet();
ExtendedExpander(SqlValidatorImpl validator, SqlValidatorScope scope,
SqlSelect select, SqlNode root, Clause clause) {
@@ -7309,6 +7313,9 @@ public class SqlValidatorImpl implements
SqlValidatorWithHints {
this.select = select;
this.root = root;
this.clause = clause;
+ if (clause == Clause.GROUP_BY) {
+ addExpandableExpressions();
+ }
}
@Override public @Nullable SqlNode visit(SqlIdentifier id) {
@@ -7317,7 +7324,7 @@ public class SqlValidatorImpl implements
SqlValidatorWithHints {
}
final boolean replaceAliases =
clause.shouldReplaceAliases(validator.config);
- if (!replaceAliases) {
+ if (!replaceAliases || (clause == Clause.GROUP_BY &&
!aliasOrdinalExpandSet.contains(id))) {
final SelectScope scope = validator.getRawSelectScopeNonNull(select);
SqlNode node = expandCommonColumn(select, id, scope, validator);
if (node != id) {
@@ -7368,24 +7375,7 @@ public class SqlValidatorImpl implements
SqlValidatorWithHints {
|| !validator.config().conformance().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;
- default:
- break;
- }
+ boolean isOrdinalLiteral = aliasOrdinalExpandSet.contains(literal);
if (isOrdinalLiteral) {
switch (literal.getTypeName()) {
case DECIMAL:
@@ -7411,6 +7401,49 @@ public class SqlValidatorImpl implements
SqlValidatorWithHints {
return super.visit(literal);
}
+ /**
+ * Add all possible expandable 'group by' expression to set, which is
+ * used to check whether expr could be expanded as alias or ordinal.
+ */
+ @RequiresNonNull({"root"})
+ private void addExpandableExpressions(@UnknownInitialization
ExtendedExpander this) {
+ switch (root.getKind()) {
+ case IDENTIFIER:
+ case LITERAL:
+ aliasOrdinalExpandSet.add(root);
+ break;
+ case GROUPING_SETS:
+ case ROLLUP:
+ case CUBE:
+ if (root instanceof SqlBasicCall) {
+ List<SqlNode> operandList = ((SqlBasicCall) root).getOperandList();
+ for (SqlNode sqlNode : operandList) {
+ addIdentifierOrdinal2ExpandSet(sqlNode);
+ }
+ }
+ break;
+ default:
+ break;
+ }
+ }
+
+ /**
+ * Identifier or literal in grouping sets, rollup, cube will be eligible
for alias.
+ *
+ * @param sqlNode expression within grouping sets, rollup, cube
+ */
+ private void addIdentifierOrdinal2ExpandSet(@UnknownInitialization
ExtendedExpander this,
+ SqlNode sqlNode) {
+ if (sqlNode.getKind() == SqlKind.ROW) {
+ List<SqlNode> rowOperandList = ((SqlCall) sqlNode).getOperandList();
+ for (SqlNode node : rowOperandList) {
+ addIdentifierOrdinal2ExpandSet(node);
+ }
+ } else if (sqlNode.getKind() == SqlKind.IDENTIFIER || sqlNode.getKind()
== SqlKind.LITERAL) {
+ aliasOrdinalExpandSet.add(sqlNode);
+ }
+ }
+
/**
* Returns whether a given node contains a {@link SqlIdentifier}.
*
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 d11d633e75..0e2e835846 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
@@ -463,6 +463,37 @@ class SqlToRelConverterTest extends SqlToRelTestBase {
.withConformance(SqlConformanceEnum.LENIENT).ok();
}
+ /**
+ * Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-4512";>[CALCITE-4512]
+ * GROUP BY expression with argument name same with SELECT field and alias
causes
+ * validation error</a>.
+ */
+ @Test void testGroupByExprArgFieldSameWithAlias() {
+ final String sql = "SELECT floor(deptno / 2) AS deptno\n"
+ + "FROM emp\n"
+ + "GROUP BY floor(deptno / 2)";
+ sql(sql)
+ .withConformance(SqlConformanceEnum.LENIENT)
+ .ok();
+ }
+
+ /**
+ * Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-4512";>[CALCITE-4512]
+ * GROUP BY expression with argument name same with SELECT field and alias
causes
+ * validation error</a>.
+ */
+ @Test void testGroupByExprArgFieldSameWithAlias2() {
+ final String sql = "SELECT deptno / 2 AS deptno, deptno / 2 as empno,
sum(sal)\n"
+ + "FROM emp\n"
+ + "GROUP BY GROUPING SETS "
+ + "((deptno), (empno, deptno / 2), (2, 1), ((1, 2), (deptno, deptno /
2)))";
+ sql(sql)
+ .withConformance(SqlConformanceEnum.LENIENT)
+ .ok();
+ }
+
@Test void testAliasInHaving() {
sql("select count(empno) as e from emp having e > 1")
.withConformance(SqlConformanceEnum.LENIENT).ok();
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 89ae082113..75bd3498ad 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -25,8 +25,12 @@ import org.apache.calcite.rel.type.RelDataTypeFactory;
import org.apache.calcite.rel.type.RelDataTypeSystem;
import org.apache.calcite.rel.type.TimeFrameSet;
import org.apache.calcite.runtime.CalciteContextException;
+import org.apache.calcite.sql.SqlBasicFunction;
import org.apache.calcite.sql.SqlCollation;
+import org.apache.calcite.sql.SqlFunction;
+import org.apache.calcite.sql.SqlFunctionCategory;
import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.SqlOperator;
import org.apache.calcite.sql.SqlOperatorTable;
@@ -37,9 +41,12 @@ import org.apache.calcite.sql.fun.SqlStdOperatorTable;
import org.apache.calcite.sql.parser.SqlParseException;
import org.apache.calcite.sql.parser.SqlParser;
import org.apache.calcite.sql.type.ArraySqlType;
+import org.apache.calcite.sql.type.OperandTypes;
+import org.apache.calcite.sql.type.ReturnTypes;
import org.apache.calcite.sql.type.SqlTypeFactoryImpl;
import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.calcite.sql.type.SqlTypeUtil;
+import org.apache.calcite.sql.util.SqlOperatorTables;
import org.apache.calcite.sql.util.SqlShuttle;
import org.apache.calcite.sql.validate.SqlAbstractConformance;
import org.apache.calcite.sql.validate.SqlConformance;
@@ -6381,6 +6388,35 @@ public class SqlValidatorTest extends
SqlValidatorTestCase {
// Group by alias with strict conformance should fail.
sql("select empno as e from emp group by ^e^")
.withConformance(strict).fails("Column 'E' not found in any table");
+
+ sql("select floor(empno/2) as empno from emp group by floor(empno/2)")
+ .withConformance(strict).ok()
+ .withConformance(lenient).ok();
+ }
+
+ /**
+ * Tests validation of alias in function within GROUP BY.
+ *
+ * @see SqlConformance#isGroupByAlias()
+ */
+ @Test void testAliasInFunctionWithinGroupBy() {
+ final SqlConformanceEnum lenient = SqlConformanceEnum.LENIENT;
+ final SqlFunction date_add =
+ SqlBasicFunction.create(SqlKind.DATE_ADD, ReturnTypes.DATE,
+ OperandTypes.STRING_INTEGER)
+ .withFunctionType(SqlFunctionCategory.TIMEDATE);
+
+ sql("select date_add('2024-01-01', empno) as empno from emp group by
^year(empno)^")
+ .withOperatorTable(
+ SqlOperatorTables.chain(
+ SqlOperatorTables.of(date_add), SqlStdOperatorTable.instance()))
+ .withConformance(lenient)
+ .fails("Cannot apply 'EXTRACT' to arguments of "
+ + "type 'EXTRACT\\(<INTERVAL YEAR> FROM <INTEGER>\\)'\\. "
+ + "Supported form\\(s\\): "
+ + "'EXTRACT\\(<DATETIME_INTERVAL> FROM <DATETIME_INTERVAL>\\)'\n"
+ + "'EXTRACT\\(<DATETIME_INTERVAL> FROM <DATETIME>\\)'\n"
+ + "'EXTRACT\\(<INTERVAL_DAY_TIME> FROM <INTERVAL_YEAR_MONTH>\\)'");
}
/**
@@ -6446,6 +6482,10 @@ public class SqlValidatorTest extends
SqlValidatorTestCase {
sql("select deptno from emp group by ^100^, deptno")
.withConformance(lenient).fails("Ordinal out of range")
.withConformance(strict).ok();
+ sql("select floor(e.deptno / 2) AS deptno from emp as e\n"
+ + "join dept as d on e.deptno = d.deptno group by ^deptno^")
+ .withConformance(strict).fails("Column 'DEPTNO' is ambiguous")
+ .withConformance(lenient).ok();
}
/** Test case for
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 f62be7d0bf..8fa9233809 100644
--- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
@@ -2409,6 +2409,45 @@ ROLLUP (deptno, job)]]>
LogicalAggregate(group=[{0, 1}], groups=[[{0, 1}, {0}, {1}, {}]],
EXPR$2=[COUNT()])
LogicalProject(DEPTNO=[$7], JOB=[$2])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ </TestCase>
+ <TestCase name="testGroupByExprArgFieldSameWithAlias">
+ <Resource name="sql">
+ <![CDATA[SELECT floor(deptno / 2) AS deptno
+FROM emp
+GROUP BY floor(deptno / 2)]]>
+ </Resource>
+ <Resource name="plan">
+ <![CDATA[
+LogicalAggregate(group=[{0}])
+ LogicalProject(DEPTNO=[FLOOR(/($7, 2))])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+]]>
+ </Resource>
+ </TestCase>
+ <TestCase name="testGroupByExprArgFieldSameWithAlias2">
+ <Resource name="sql">
+ <![CDATA[SELECT deptno / 2 AS deptno, deptno / 2 as empno, sum(sal)
+FROM emp
+GROUP BY GROUPING SETS ((deptno), (empno, deptno / 2), (2, 1), ((1, 2),
(deptno, deptno / 2)))]]>
+ </Resource>
+ <Resource name="plan">
+ <![CDATA[
+LogicalProject(DEPTNO=[$0], EMPNO=[$0], EXPR$2=[$1])
+ LogicalUnion(all=[true])
+ LogicalAggregate(group=[{0}], EXPR$2=[SUM($1)])
+ LogicalProject(EMPNO=[/($7, 2)], SAL=[$5])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalAggregate(group=[{0}], EXPR$2=[SUM($1)])
+ LogicalProject(EMPNO=[/($7, 2)], SAL=[$5])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalAggregate(group=[{0}], EXPR$2=[SUM($1)])
+ LogicalProject(EMPNO=[/($7, 2)], SAL=[$5])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalAggregate(group=[{0}], EXPR$2=[SUM($1)])
+ LogicalProject(EMPNO=[/($7, 2)], SAL=[$5])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
</Resource>
</TestCase>
diff --git a/core/src/test/resources/sql/misc.iq
b/core/src/test/resources/sql/misc.iq
index 40bcc82ef9..08ecc00a6b 100644
--- a/core/src/test/resources/sql/misc.iq
+++ b/core/src/test/resources/sql/misc.iq
@@ -2286,4 +2286,22 @@ FROM "hr"."emps";
!ok
+!use post
+
+# [CALCITE-4512] GROUP BY expression with argument name same with SELECT field
and alias causes validation error
+SELECT floor(empno/2) as empno
+FROM emps
+GROUP BY floor(empno/2);
++-------+
+| EMPNO |
++-------+
+| 50 |
+| 55 |
+| 60 |
+| 65 |
++-------+
+(4 rows)
+
+!ok
+
# End misc.iq
