vlsi commented on a change in pull request #2329:
URL: https://github.com/apache/calcite/pull/2329#discussion_r562015869



##########
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:
       The problem with `e1 == null ? null` is that behavior is not documented, 
and it is not tested (there's no test which checks what happens in case 
user-provided `keySelector` wants to convert `null` to something non-`null`).
   
   `mergeUnion` method is already very generic (it has customization for 
`keySelector`, `comparator`), so I see no reasons why the method should have 
its own `null-handling`. It is generic enough to let users decide what they 
want to do with nulls.




----------------------------------------------------------------
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]


Reply via email to