neilconway commented on code in PR #23636:
URL: https://github.com/apache/datafusion/pull/23636#discussion_r3632968290
##########
datafusion/common/src/functional_dependencies.rs:
##########
@@ -498,10 +494,15 @@ pub fn aggregate_functional_dependencies(
// GROUP BY expressions come here as a prefix.
item.source_indices.iter().all(|idx| idx < &count)
}) {
- // Add a new functional dependency associated with the whole table:
- // Use nullable property of the GROUP BY expression:
+ // Add a new functional dependency associated with the whole table.
+ //
+ // `nullable` is `false`: a nullable dependence means multiple NULL
+ // keys may coexist (as under a SQL UNIQUE constraint, where NULLs
+ // compare distinct). That cannot happen after grouping: GROUP BY
+ // treats NULLs as equal, so every key combination -- NULL included
+ // -- occurs in exactly one output row, like a primary key.
Review Comment:
Sure, that's fine -- I think a more focused unit test (in addition to the
existing SLT) would be nice to have but I don't think it's a blocker.
--
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]
