hsyuan commented on a change in pull request #2006:
URL: https://github.com/apache/calcite/pull/2006#discussion_r439782434



##########
File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeJoin.java
##########
@@ -222,6 +277,30 @@ public static boolean isMergeJoinSupported(JoinRelType 
joinType) {
     return mapping;
   }
 
+  private RelCollation extendCollation(RelCollation collation, List<Integer> 
keys) {
+    List<RelFieldCollation> fieldsForNewCollation = new 
ArrayList<>(keys.size());
+    fieldsForNewCollation.addAll(collation.getFieldCollations());
+    Set<Integer> keySet = new HashSet<>(keys);

Review comment:
       Do we need to keep the keys order?

##########
File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeJoin.java
##########
@@ -134,31 +137,83 @@ public static boolean isMergeJoinSupported(JoinRelType 
joinType) {
       final RelTraitSet required) {
     // Required collation keys can be subset or superset of merge join keys.
     RelCollation collation = required.getCollation();
+    int leftInputFieldCount = left.getRowType().getFieldCount();
+
     List<Integer> reqKeys = RelCollations.ordinals(collation);
-    ImmutableBitSet reqKeySet = ImmutableBitSet.of(reqKeys);
+    List<Integer> leftKeys = joinInfo.leftKeys.toIntegerList();
+    List<Integer> rightKeys =
+        joinInfo.rightKeys.incr(leftInputFieldCount).toIntegerList();
+    List<Integer> rightKeysNotShifted = joinInfo.rightKeys.toIntegerList();

Review comment:
       Why `toIntegerList`?

##########
File path: 
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeJoin.java
##########
@@ -222,6 +277,30 @@ public static boolean isMergeJoinSupported(JoinRelType 
joinType) {
     return mapping;
   }
 
+  private RelCollation extendCollation(RelCollation collation, List<Integer> 
keys) {
+    List<RelFieldCollation> fieldsForNewCollation = new 
ArrayList<>(keys.size());
+    fieldsForNewCollation.addAll(collation.getFieldCollations());
+    Set<Integer> keySet = new HashSet<>(keys);
+    for (RelFieldCollation rf : collation.getFieldCollations()) {
+      keySet.remove(rf.getFieldIndex());
+    }
+    for (Integer i : keySet) {
+      fieldsForNewCollation.add(new RelFieldCollation(i));
+    }
+    return RelCollations.of(fieldsForNewCollation);
+  }
+
+  private RelCollation removeCollationFieldsNotOnJoinKey(

Review comment:
       It is not obvious to get the meaning from method name. An example and 
comment will help. 




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