findepi commented on code in PR #16538:
URL: https://github.com/apache/datafusion/pull/16538#discussion_r2167033810


##########
datafusion/sql/src/expr/function.rs:
##########
@@ -375,6 +375,10 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
                     return plan_err!("WITHIN GROUP clause is required when 
calling ordered set aggregate function({})", fm.name());
                 }
 
+                if !fm.is_ordered_set_aggregate() && !within_group.is_empty() {
+                    return plan_err!("WITHIN GROUP clause is not permitted for 
non-ordered set aggregate function({})", fm.name());

Review Comment:
   array_agg is definitely ordered, or order-sensitive



##########
datafusion/sqllogictest/test_files/aggregate.slt:
##########
@@ -7040,3 +7040,7 @@ VALUES
 ) GROUP BY 1 ORDER BY 1;
 ----
 x 1
+
+statement error DataFusion error:  Error during planning: WITHIN GROUP clause 
is not permitted for non-ordered set aggregate function\(array_agg\)
+SELECT array_agg(DISTINCT a_varchar) within group (order by a_varchar)
+FROM (VALUES ('a'), ('d'), ('c'), ('a')) t(a_varchar);

Review Comment:
   There is no reason for this not to work. Equivalent syntax is already 
supported:
   
   ```
   DataFusion CLI v48.0.0
   > SELECT array_agg(DISTINCT a_varchar order by a_varchar)
   FROM (VALUES ('a'), ('d'), ('c'), ('a')) t(a_varchar);
   +-----------------------------------------------------------------------+
   | array_agg(DISTINCT t.a_varchar) ORDER BY [t.a_varchar ASC NULLS LAST] |
   +-----------------------------------------------------------------------+
   | [a, c, d]                                                             |
   +-----------------------------------------------------------------------+
   ```
   
   ... and not only in DataFusion
   
   ```
   trino> SELECT array_agg(DISTINCT a_varchar order by a_varchar)
       -> FROM (VALUES ('a'), ('d'), ('c'), ('a')) t(a_varchar);;
      _col0
   -----------
    [a, c, d]
   ```
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to