rubenada commented on code in PR #3311:
URL: https://github.com/apache/calcite/pull/3311#discussion_r1314585771
##########
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:
I admit it is a hacky way to use comparators, and indeed it breaks its
semantic, but I was not able to find a better way to fix this situation.
> Since we also use the same comparator to compare records from same side,
it may be worth to also test the case that multi null key records from one side?
I'm not sure I understand... could you please provide an example?
##########
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:
I admit it is a hacky way to use comparators, and indeed it breaks its
semantic, but I was not able to find a better way to fix this situation.
> Since we also use the same comparator to compare records from same side,
it may be worth to also test the case that multi null key records from one side?
I'm not sure I understand... could you please provide an example?
--
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]
