You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/09/10 06:31:13 UTC

[GitHub] [pinot] 61yao commented on a diff in pull request #9350: [multistage][bug-fix] Fix the join condition order different from join table order

61yao commented on code in PR #9350:
URL: https://github.com/apache/pinot/pull/9350#discussion_r967605726


##########
pinot-query-planner/src/main/java/org/apache/calcite/rel/rules/PinotJoinExchangeNodeInsertRule.java:
##########
@@ -77,12 +77,12 @@ public void onMatch(RelOptRuleCall call) {
       rightExchange = LogicalExchange.create(rightInput, RelDistributions.BROADCAST_DISTRIBUTED);
     } else { // if (hints.contains(PinotRelationalHints.USE_HASH_DISTRIBUTE)) {
       RexCall joinCondition = (RexCall) join.getCondition();

Review Comment:
   Done. Can you double check it works for Equal case. The test cases pass but I am not fully sure. 



##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/PlannerUtils.java:
##########
@@ -44,22 +44,31 @@ private PlannerUtils() {
     // do not instantiate.
   }
 
-  public static List<List<Integer>> getJoinKeyFromConditions(RexCall joinCondition, int leftNodeOffset) {
+  public static List<List<Integer>> getJoinKeyFromConditions(RexCall joinCondition, int leftNodeOffset,
+      int rightNodeOffset) {
     switch (joinCondition.getOperator().getKind()) {
       case EQUALS:
         RexNode left = joinCondition.getOperands().get(0);
         RexNode right = joinCondition.getOperands().get(1);
+        int leftIndex = ((RexInputRef) left).getIndex();
+        int rightIndex = ((RexInputRef) right).getIndex();
+        if (left.hashCode() > right.hashCode()) {
+          // right condition is before left condition.
+          leftIndex -= rightNodeOffset;
+        } else {
+          rightIndex -= leftNodeOffset;
+        }
         Preconditions.checkState(left instanceof RexInputRef, "only reference supported");
         Preconditions.checkState(right instanceof RexInputRef, "only reference supported");
-        return Arrays.asList(Collections.singletonList(((RexInputRef) left).getIndex()),
-            Collections.singletonList(((RexInputRef) right).getIndex() - leftNodeOffset));
+        return Arrays.asList(Collections.singletonList(leftIndex), Collections.singletonList(rightIndex));
       case AND:
         List<List<Integer>> predicateColumns = new ArrayList<>(2);
         predicateColumns.add(new ArrayList<>());
         predicateColumns.add(new ArrayList<>());
         for (RexNode operand : joinCondition.getOperands()) {
           Preconditions.checkState(operand instanceof RexCall);
-          List<List<Integer>> subPredicate = getJoinKeyFromConditions((RexCall) operand, leftNodeOffset);
+          List<List<Integer>> subPredicate =
+              getJoinKeyFromConditions((RexCall) operand, leftNodeOffset, rightNodeOffset);
           predicateColumns.get(0).addAll(subPredicate.get(0));
           predicateColumns.get(1).addAll(subPredicate.get(1));

Review Comment:
   Done.



-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org