rubenada commented on code in PR #3311: URL: https://github.com/apache/calcite/pull/3311#discussion_r1315141021
########## linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java: ########## @@ -4495,6 +4492,27 @@ private boolean advanceRight(TInner right, TKey rightKey) { } } + public static int compareNullsLastForMergeJoin(@Nullable Comparable v0, @Nullable Comparable v1) { + return compareNullsLastForMergeJoin(v0, v1, null); + } + + public static int compareNullsLastForMergeJoin(@Nullable Comparable v0, @Nullable Comparable v1, + @Nullable Comparator comparator) { + // Special code for mergeJoin algorithm: in case of two null values, they must not be + // considered as equal (otherwise the join would return incorrect results); instead, consider + // the first (left) value as "bigger", to advance on the right value and continue with the + // algorithm + if (v0 == null && v1 == null) { + return 1; Review Comment: @libenchao I have committed a new approach that should handle this issue: - Use a (control flow) BothValuesAreNullException in the Comparator to avoid breaking its semantics - Use an EqualityComparer (i.e. nulls are considered equal) to compare keys from the same side wdyt? UPDATE: I have committed new tests for MergeJoin (which used to fail with IllegalStateException with the old approach, and now pass fine). -- 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: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org