zabetak commented on code in PR #4089:
URL: https://github.com/apache/calcite/pull/4089#discussion_r1882514275
##########
core/src/main/java/org/apache/calcite/rel/metadata/RelMdUniqueKeys.java:
##########
@@ -184,10 +213,14 @@ private static Set<ImmutableBitSet>
getProjectUniqueKeys(SingleRel rel, RelMetad
// the resulting unique keys would be {{0},{4}}, {{1},{4}}
Iterable<List<Integer>> product = Linq4j.product(Util.transform(colMask,
mapInToOutPos::get));
-
- resultBuilder.addAll(Util.transform(product, ImmutableBitSet::of));
+ for (List<Integer> passKey : product) {
Review Comment:
The loop just iterates over the result of the cross product. If the cross
product is deterministic so is the iteration.
On the other hand the choice between `ImmutableSet` vs. `HashSet` does have
implications on the deterministic iteration since the latter provides no
guarantees. If we want the limit to be precise it would be difficult to
continue using the ImmutableSet API. If we relax the limit accuracy then
probably we can keep it as before.
Note, that even before my changes there are various parts inside this
handler that do not guaranteed that results are deterministic (usage of
`HashSet`, `stream()`, etc.) so I am not sure how strictly we want to enforce
this property.
--
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]
