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 788a4404a8c3fe72983f865f2d5af51dce04aa72
Author: Itiel Sadeh <[email protected]>
AuthorDate: Tue Apr 18 10:09:29 2023 +0300

    [CALCITE-5653] SELECT DISTINCT aggregate function with ORDER BY gives 
invalid validation error
    
    For example, the query
    
      SELECT distinct sum(deptno + '1') FROM dept ORDER BY 1
    
    throws
    
      Expression 'SUM(`DEPT`.`DEPTNO` + CAST('1' AS INTEGER))'
      is not in the select clause
    
    The fix is for AggChecker to look in the expanded select list.
    
    Close apache/calcite#3160
---
 .../org/apache/calcite/sql/validate/AggChecker.java  | 20 +++++++++++---------
 .../org/apache/calcite/test/SqlValidatorTest.java    |  6 ++++++
 core/src/test/resources/sql/agg.iq                   | 11 +++++++++++
 3 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/sql/validate/AggChecker.java 
b/core/src/main/java/org/apache/calcite/sql/validate/AggChecker.java
index 170411eafe..9a93718c38 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/AggChecker.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/AggChecker.java
@@ -20,7 +20,6 @@ import org.apache.calcite.sql.SqlCall;
 import org.apache.calcite.sql.SqlIdentifier;
 import org.apache.calcite.sql.SqlKind;
 import org.apache.calcite.sql.SqlNode;
-import org.apache.calcite.sql.SqlNodeList;
 import org.apache.calcite.sql.SqlSelect;
 import org.apache.calcite.sql.SqlWindow;
 import org.apache.calcite.sql.fun.SqlStdOperatorTable;
@@ -33,7 +32,6 @@ import java.util.ArrayDeque;
 import java.util.Deque;
 import java.util.List;
 
-import static 
org.apache.calcite.sql.validate.SqlNonNullableAccessors.getSelectList;
 import static org.apache.calcite.util.Static.RESOURCE;
 
 import static java.util.Objects.requireNonNull;
@@ -142,12 +140,18 @@ class AggChecker extends SqlBasicVisitor<Void> {
   }
 
   @Override public Void visit(SqlCall call) {
-    final SqlValidatorScope scope = scopes.peek();
+    final SqlValidatorScope scope =
+        requireNonNull(scopes.peek(), () -> "scope for " + call);
     if (call.getOperator().isAggregator()) {
       if (distinct) {
         if (scope instanceof AggregatingSelectScope) {
-          SqlNodeList selectList =
-              getSelectList((SqlSelect) scope.getNode());
+          final SqlSelect select = (SqlSelect) scope.getNode();
+          SelectScope selectScope =
+              requireNonNull(validator.getRawSelectScope(select),
+                  () -> "rawSelectScope for " + scope.getNode());
+          List<SqlNode> selectList =
+              requireNonNull(selectScope.getExpandedSelectList(),
+                  () -> "expandedSelectList for " + selectScope);
 
           // Check if this aggregation function is just an element in the 
select
           for (SqlNode sqlNode : selectList) {
@@ -196,8 +200,7 @@ class AggChecker extends SqlBasicVisitor<Void> {
       } else if (over instanceof SqlIdentifier) {
         // Check the corresponding SqlWindow in WINDOW clause
         final SqlWindow window =
-            requireNonNull(scope, () -> "scope for " + call)
-                .lookupWindow(((SqlIdentifier) over).getSimple());
+            scope.lookupWindow(((SqlIdentifier) over).getSimple());
         requireNonNull(window, () -> "window for " + call);
         window.getPartitionList().accept(this);
         window.getOrderList().accept(this);
@@ -234,8 +237,7 @@ class AggChecker extends SqlBasicVisitor<Void> {
     }
 
     // Switch to new scope.
-    SqlValidatorScope newScope = requireNonNull(scope, () -> "scope for " + 
call)
-        .getOperandScope(call);
+    SqlValidatorScope newScope = scope.getOperandScope(call);
     scopes.push(newScope);
 
     // Visit the operands (only expressions).
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 f84a1b1b79..f0f7395e7b 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -5903,6 +5903,12 @@ public class SqlValidatorTest extends 
SqlValidatorTestCase {
         + " order by upper(^eno^)")
         .failsIf(!conformance.isSortByAlias(),
             "Column 'ENO' not found in any table");
+
+    // Test case for [CALCITE-5653], order by on aggregate will fail when 
there is an implicit cast
+    // and IdentifierExpansion is off
+    sql("select distinct sum(deptno + '1') from dept order by 1")
+        .withValidatorIdentifierExpansion(false)
+        .ok();
   }
 
   /**
diff --git a/core/src/test/resources/sql/agg.iq 
b/core/src/test/resources/sql/agg.iq
index 1210727959..f359d06de6 100644
--- a/core/src/test/resources/sql/agg.iq
+++ b/core/src/test/resources/sql/agg.iq
@@ -3585,5 +3585,16 @@ from emp;
 +----+----+-------+
 (1 row)
 
+!ok
+
+# DISTINCT query with ORDER BY on aggregate when there is an implicit cast
+select distinct sum(deptno + '1') as deptsum from dept order by 1;
++---------+
+| DEPTSUM |
++---------+
+|     104 |
++---------+
+(1 row)
+
 !ok
 # End agg.iq

Reply via email to