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

[GitBox <gi...@apache.org>]

walterddr commented on code in PR #9350:
URL: https://github.com/apache/pinot/pull/9350#discussion_r967340007


##########
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:
   this is probably my hack previously done. we might not need these since it is already supported by calcite somewhere



##########
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:
   so instead of getting this from `join.getCondition()` you cando
   ```
       JoinInfo joinInfo = join.analyzeCondition();
       RelNode leftInput = join.getInput(0);
       RelNode rightInput = join.getInput(1);
       RexNode eqCondition = joinInfo.getEquiCondition(leftInput, rightInput, join.getCluster().getRexBuilder());
   ```
   this eqCondition already creates out the join key input reference (in left/right order)



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