alamb commented on code in PR #23636: URL: https://github.com/apache/datafusion/pull/23636#discussion_r3633475576
########## datafusion/sqllogictest/test_files/group_by.slt: ########## @@ -5694,3 +5694,86 @@ drop table users_with_pk; statement ok drop table user_orders; + +# DISTINCT must not be removed based on a nullable UNIQUE constraint: SQL +# UNIQUE permits multiple NULL keys, but DISTINCT treats NULLs as equal and +# must still collapse them. +statement ok +CREATE TABLE t_nullable_unique (x INT UNIQUE) AS VALUES (NULL), (NULL), (1); + +query I +SELECT DISTINCT x FROM t_nullable_unique ORDER BY x NULLS LAST; +---- +1 +NULL + +query TT +EXPLAIN SELECT DISTINCT x FROM t_nullable_unique; +---- +logical_plan +01)Aggregate: groupBy=[[t_nullable_unique.x]], aggr=[[]] +02)--TableScan: t_nullable_unique projection=[x] +physical_plan +01)AggregateExec: mode=FinalPartitioned, gby=[x@0 as x], aggr=[] +02)--RepartitionExec: partitioning=Hash([x@0], 4), input_partitions=1 +03)----AggregateExec: mode=Partial, gby=[x@0 as x], aggr=[] +04)------DataSourceExec: partitions=1, partition_sizes=[1] + +# Grouping, on the other hand, collapses the multiple NULL keys that the +# UNIQUE constraint permitted, so the GROUP BY output *is* unique on x +# (NULL included). A DISTINCT over it is a no-op and can be removed: only +# the inner Aggregate remains in the plan. +query I +SELECT DISTINCT x FROM (SELECT x FROM t_nullable_unique GROUP BY x) ORDER BY x NULLS LAST; +---- +1 +NULL + +query TT +EXPLAIN SELECT DISTINCT x FROM (SELECT x FROM t_nullable_unique GROUP BY x); +---- +logical_plan +01)Aggregate: groupBy=[[t_nullable_unique.x]], aggr=[[]] +02)--TableScan: t_nullable_unique projection=[x] +physical_plan +01)AggregateExec: mode=FinalPartitioned, gby=[x@0 as x], aggr=[] +02)--RepartitionExec: partitioning=Hash([x@0], 4), input_partitions=1 +03)----AggregateExec: mode=Partial, gby=[x@0 as x], aggr=[] +04)------DataSourceExec: partitions=1, partition_sizes=[1] + +statement ok +drop table t_nullable_unique; + +# When the nullable UNIQUE column covers only part of the GROUP BY key, the +# derived dependency stays nullable: rows (NULL, 1) and (NULL, 2) are legal +# under `x UNIQUE` and produce two output rows with x = NULL, so x alone is +# not a key of the GROUP BY output and the DISTINCT must be kept. +statement ok +CREATE TABLE t_nullable_unique_multi (x INT UNIQUE, y INT) AS VALUES (NULL, 1), (NULL, 2), (1, 3); + +# NOTE: `rowsort` rather than `ORDER BY x, y` because `ORDER BY` over a +# nullable UNIQUE column currently drops the trailing sort keys (see +# `get_required_sort_exprs_indices`), which is a separate bug. Review Comment: This is a mess -- and my claude found a bunch of other problems too. I think we need to add some test coverage... Working on that in the background with claude -- 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]
