raams95 opened a new pull request, #26440: URL: https://github.com/apache/datafusion/pull/26440
## Which issue does this PR close? - Closes #ISSUE_NUMBER. ## Rationale for this change `array_agg(DISTINCT x ORDER BY x)` returns its values in the wrong order when an earlier aggregate in the same query orders by `x` in the other direction: ```sql SELECT array_agg(x ORDER BY x DESC), array_agg(DISTINCT x ORDER BY x) FROM (VALUES (2), (NULL), (1), (2)) t(x); -- before: [NULL, 2, 2, 1] [NULL, 2, 1] -- after: [NULL, 2, 2, 1] [1, 2, NULL] ``` `string_agg(DISTINCT x, ',' ORDER BY x)` has the same problem, since it uses the same accumulator. When the aggregates need opposite orderings, the planner swaps one of them for its reverse expression, which flips the `ORDER BY` and sets `is_reversed`. The ordered (non-`DISTINCT`) `array_agg` accumulator undoes that at the end, but the `DISTINCT` accumulator sorts its values itself using the flipped sort options and never looks at `is_reversed`. ## What changes are included in this PR? In `ArrayAgg::accumulator`, the `DISTINCT` branch flips the sort options back when `is_reversed` is set, so `DistinctArrayAggAccumulator` sorts in the order the user asked for. ## What is the testing strategy for this PR? New sqllogictest cases: - `array_agg.slt`: the query above (including a NULL, so NULL placement is checked too) and a `GROUP BY` variant. - `aggregate.slt`: `string_agg(DISTINCT ... ORDER BY ...)` next to a `string_agg(... ORDER BY ... DESC)`. All three fail on `main` and pass with this change. The rest of the sqllogictest suite and the `datafusion-functions-aggregate` unit tests pass. ## Are there any user-facing changes? Queries like the one above now return the documented order. No API changes. ## Other notes #26357 adds a `GroupsAccumulator` for `array_agg(DISTINCT)`. If it sorts with the expression's sort options, the same `is_reversed` handling is probably needed there too. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
