vlsi commented on a change in pull request #2329:
URL: https://github.com/apache/calcite/pull/2329#discussion_r561933483
##########
File path:
linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
##########
@@ -4683,4 +4683,191 @@ private void flush() {
}
};
}
+
+ /**
+ * Merge Union Enumerable.
+ * Performs a union (or union all) of all its inputs (which must be already
sorted),
+ * respecting the order.
+ *
+ * @param sources input enumerables (must be already sorted)
+ * @param sortKeySelector sort key selector
+ * @param sortComparator sort comparator to decide the next item
+ * @param all whether duplicates will be considered or not
+ * @param equalityComparer {@link EqualityComparer} to control duplicates,
+ * only used if {@code all} is {@code false}
+ * @param <TSource> record type
+ * @param <TKey> sort key
+ */
+ public static <TSource, @Nullable TKey> Enumerable<TSource> mergeUnion(
Review comment:
```java
private int compare(TSource e1, TSource e2) {
final TKey key1 = e1 == null ? null : this.sortKeySelector.apply(e1);
final TKey key2 = e2 == null ? null : this.sortKeySelector.apply(e2);
return this.sortComparator.compare(key1, key2);
}
```
Ok, what does the error mean here?
It says that the comparison key can become `null` even in the case
`sortKeySelector` never returns nulls. I believe if the user passes
`sortKeySelector` which never returns nulls, they won't expect that
`mergeUnion` would silently skip `keySelector`.
Unfortunately, you do not declare the behavior with regards to the `null`
values for `TSource` elements in `List<Enumerable<TSource>> sources`.
---
I believe the way to fix this is to remove custom `null` handling and just
pass the keys to `sortKeySelector` as is:
```java
private int compare(TSource e1, TSource e2) {
final TKey key1 = this.sortKeySelector.apply(e1);
final TKey key2 = this.sortKeySelector.apply(e2);
return this.sortComparator.compare(key1, key2);
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
