PaulJackson123 commented on code in PR #3495:
URL: https://github.com/apache/calcite/pull/3495#discussion_r1399393791
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdColumnUniqueness.java:
##########
@@ -67,6 +70,9 @@ public class RelMdColumnUniqueness
public static final RelMetadataProvider SOURCE =
ReflectiveRelMetadataProvider.reflectiveSource(
new RelMdColumnUniqueness(),
BuiltInMetadata.ColumnUniqueness.Handler.class);
+ static final Set<SqlKind> PASSTHROUGH_AGGREGATIONS =
Review Comment:
> You should not apply this 'passthrough' logic to aggregate functions with
a filter. `MIN(x) FILTER (WHERE y > 10)` is not necessarily unique even if `x`
is unique. (Because `y > 10` may be false for all rows, and therefore it could
return NULL.)
I coded this PR with the understanding that there is a distinction between a
primary key (non-null) and a unique column (allowing a single null). However, I
took this understanding from the behavior of MS SQL Server, and, apparently,
it's not consistent with the spec, which, according to [this
link](https://stackoverflow.com/a/767702/957595), allows _multiple NULLs_ in a
unique column.
I think this means the first question is what is allowed in unique columns:
a) no nulls, b) at most one null, c) multiple nulls.
If we conclude it's either b) or c), then I think we're safe with the
`FILTER` because it can result in at most a single null.
--
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]
