kosiew commented on code in PR #22865:
URL: https://github.com/apache/datafusion/pull/22865#discussion_r3386024891


##########
datafusion/physical-plan/src/joins/nested_loop_join.rs:
##########
@@ -2368,25 +2434,20 @@ impl NestedLoopJoinStream {
 
         // Early return if join type can't have unmatched rows
         let join_type_no_produce_left = 
!need_produce_result_in_final(self.join_type);
-        // Early return if another thread is already processing unmatched rows.
+        // Early return if another partition is the designated emitter.

Review Comment:
   Nice simplification. Since the invariant is already captured by `ProbeEnd` 
and documented on the state/helper, I wonder if this comment and the negated 
temporary could be trimmed down a bit.
   
   Something along these lines seems a little easier to scan:
   
   ```rust
   // `ProbeEnd` already recorded whether this stream emits unmatched-left rows.
   let finished = self.left_emit_idx >= left_batch.num_rows();
   if join_type_no_produce_left || !self.is_unmatched_left_emitter || finished {
       return Ok(false);
   }
   ```
   
   That keeps the emit state focused on the local decision while preserving the 
existing behavior.



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to