kosiew commented on code in PR #23684:
URL: https://github.com/apache/datafusion/pull/23684#discussion_r3681822780
##########
datafusion/sqllogictest/test_files/aggregates_topk.slt:
##########
@@ -585,3 +587,191 @@ drop table ids;
statement ok
drop table traces;
+
+#######
+# Regression tests for all-NULL groups in TopK aggregation (issues #23440,
#22190):
+# a group whose aggregate inputs are all NULL must be emitted with a NULL
+# aggregate value instead of disappearing from the result
+#######
+statement ok
+CREATE TABLE t0 AS SELECT * FROM (VALUES ('gamma', CAST(NULL AS DOUBLE))) v(s,
y);
+
+# MIN/MAX with NULLS FIRST must use regular aggregation because a group's
+# aggregate can transition from NULL to non-NULL and worsen its rank.
+query TT
+explain select s, max(y) as max_y from t0 group by s order by max_y desc nulls
first limit 3;
+----
+logical_plan
+01)Sort: max_y DESC NULLS FIRST, fetch=3
+02)--Projection: t0.s, max(t0.y) AS max_y
+03)----Aggregate: groupBy=[[t0.s]], aggr=[[max(t0.y)]]
+04)------TableScan: t0 projection=[s, y]
+physical_plan
+01)ProjectionExec: expr=[s@0 as s, max(t0.y)@1 as max_y]
+02)--SortExec: TopK(fetch=3), expr=[max(t0.y)@1 DESC],
preserve_partitioning=[false]
+03)----AggregateExec: mode=SinglePartitioned, gby=[s@0 as s], aggr=[max(t0.y)]
+04)------DataSourceExec: partitions=1, partition_sizes=[1]
+
+# issue #23440: single all-NULL group, MAX DESC NULLS FIRST LIMIT 3
+query R
+SELECT max_y FROM (SELECT s, MAX(y) AS max_y FROM t0 GROUP BY s) ORDER BY
max_y DESC NULLS FIRST LIMIT 3;
+----
+NULL
+
+# issue #22190: single all-NULL group, MIN ASC NULLS LAST LIMIT 20
+query R
+SELECT min_y FROM (SELECT s, MIN(y) AS min_y FROM t0 GROUP BY s) ORDER BY
min_y ASC NULLS LAST LIMIT 20;
Review Comment:
The NULLS LAST all-NULL regression should still exercise bounded TopK
aggregation, but this test currently checks only the final result. Could we add
a nearby EXPLAIN assertion that verifies `AggregateExec ... lim=[...]` for one
NULLS LAST all-NULL or mixed case? That would help ensure this coverage does
not accidentally pass through the regular aggregation fallback.
--
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]
