Re: [PR] IGNITE-27660 : Revise usage of `checkState()` in the join nodes. [ignite]

["alex-plekhanov (via GitHub)" <gi...@apache.org>]

alex-plekhanov commented on code in PR #12680:
URL: https://github.com/apache/ignite/pull/12680#discussion_r2759178348


##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/HashJoinNode.java:
##########
@@ -186,8 +186,11 @@ protected Iterator<Row> untouched() {
             hashStore.computeIfAbsent(key, k -> new TouchedArrayList<>()).add(row);
         }
 
-        if (waitingRight == 0)
+        if (waitingRight == 0) {
+            checkState();

Review Comment:
   Please, merge the master



##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/NestedLoopJoinNode.java:
##########
@@ -456,16 +463,15 @@ public FullOuterJoin(
                 assert lastPushedInd >= 0;
 
                 inLoop = true;
+                Row emptyLeft = leftRowFactory.create();
                 try {
                     for (lastPushedInd = rightNotMatchedIndexes.nextSetBit(lastPushedInd);;
                         lastPushedInd = rightNotMatchedIndexes.nextSetBit(lastPushedInd + 1)
                     ) {
-                        checkState();
-
                         if (lastPushedInd < 0)
                             break;
 
-                        Row row = rowHnd.concat(leftRowFactory.create(), rightMaterialized.get(lastPushedInd));

Review Comment:
   Reschedule check here?



##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/CorrelatedNestedLoopJoinNode.java:
##########
@@ -434,6 +433,14 @@ private void join() throws Exception {
 
                 try {
                     while (requested > 0 && notMatchedIdx < leftInBuf.size()) {
+                        if (processed++ > IN_BUFFER_SIZE) {
+                            rightIdx = preservedRightIdx;
+
+                            context().execute(this::join0, this::onError);
+
+                            return;
+                        }
+

Review Comment:
   Using preservedRightIdx a little bit confusing. I think it's more correct to set rightIdx to 0 after the loop. But, looks like this whole block is redundant, leftInBuf have only one item while CorrelatedNestedLoopJoinRule.INSTANCE_BATCHED is disabled (currently disabled). And after enabling - it can't produce more than 100 items, so it's not a big problem if we process IN_BUFFER_SIZE or IN_BUFFER_SIZE + 100 rows. Perhaps we can leave only processed rows increment here, without rescheduling.



##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/NestedLoopJoinNode.java:
##########
@@ -323,16 +327,18 @@ public RightJoin(
                 assert lastPushedInd >= 0;
 
                 inLoop = true;
+                Row emptyLeft = leftRowFactory.create();

Review Comment:
   Empty row should be created in constructor. Here it can be created redundantly (when all right row match left rows).



-- 
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: notifications-unsubscribe@ignite.apache.org

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