[CALCITE-4512] GROUP BY expression with argument name same with SELECT field and alias causes validation error

core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java

@@ -127,10 +127,12 @@
 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;
 
@@ -7269,13 +7271,18 @@ static class ExtendedExpander extends Expander {
     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) {
       super(validator, scope);
       this.select = select;
       this.root = root;
       this.clause = clause;
+      if (clause == Clause.GROUP_BY) {
+        addExpandableExpressions();
+      }
     }
 
     @Override public @Nullable SqlNode visit(SqlIdentifier id) {
@@ -7284,7 +7291,7 @@ static class ExtendedExpander extends Expander {
       }
 
       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) {
@@ -7335,24 +7342,7 @@ static class ExtendedExpander extends Expander {
           || !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:
@@ -7378,6 +7368,49 @@ static class ExtendedExpander extends Expander {
       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}.
      *

core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java

@@ -443,6 +443,37 @@ public static void checkActualAndReferenceFiles() {
         .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();

core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java

@@ -25,8 +25,12 @@
 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.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;
@@ -6289,6 +6296,35 @@ public boolean isBangEqualAllowed() {
     // 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>\\)'");
   }
 
   /**
@@ -6354,6 +6390,10 @@ public boolean isBangEqualAllowed() {
     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

core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml

@@ -2310,6 +2310,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>

core/src/test/resources/sql/misc.iq

@@ -2658,4 +2658,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