kumarUjjawal commented on code in PR #23957:
URL: https://github.com/apache/datafusion/pull/23957#discussion_r3735838661


##########
datafusion/physical-plan/src/joins/hash_join/stream.rs:
##########
@@ -894,14 +912,26 @@ impl HashJoinStream {
             last_joined_right_idx.map_or(0, |v| v + 1)
         };
 
-        let (left_indices, right_indices) = adjust_indices_by_join_type(
+        let (left_indices, mut right_indices) = adjust_indices_by_join_type(
             left_indices,
             right_indices,
             index_alignment_range_start..index_alignment_range_end,
             self.join_type,
             self.right_side_ordered,
         )?;
 
+        // If null-aware RightAnti join, we don't want to emit NULL probe keys

Review Comment:
   always allocates a Vec and copies every unmatched index, even when the probe 
batch has no NULLs. This affects low-hit NOT IN queries that return many 
rows—the important target workload. Reuse the logical validity mask and skip 
the copy when it contains no NULLs.



##########
datafusion/physical-plan/src/joins/hash_join/stream.rs:
##########
@@ -742,40 +742,58 @@ impl HashJoinStream {
         let timer = self.join_metrics.join_time.timer();
 
         // Null-aware anti join semantics:
+
         // For LeftAnti: output LEFT (build) rows where LEFT.key NOT IN 
RIGHT.key
         // 1. If RIGHT (probe) contains NULL in any batch, no LEFT rows should 
be output
         // 2. LEFT rows with NULL keys should not be output (handled in final 
stage)
+
+        // For RightAnti: output RIGHT (probe) rows where RIGHT.key NOT IN 
LEFT.key
+        // 1. If LEFT (build) contains NULL, no RIGHT rows should be output
+        // 2. RIGHT rows with NULL keys should not be output
+        // 3. If LEFT (build) is empty, all RIGHT rows should be output
         if self.null_aware {
-            // Mark that we've seen a probe batch with actual rows (probe side 
is non-empty)
-            // Only set this if batch has rows - empty batches don't count
-            // Use shared atomic state so all partitions can see this global 
information
-            if state.batch.num_rows() > 0 {
-                build_side
-                    .left_data
-                    .probe_side_non_empty
-                    .store(true, Ordering::Relaxed);
-            }
+            match self.join_type {
+                JoinType::RightAnti => {

Review Comment:
   filtered null-aware joins should not beswapped or filtered null-aware 
RightAnti should be rejected.



##########
datafusion/physical-plan/src/joins/hash_join/exec.rs:
##########
@@ -2330,6 +2338,8 @@ async fn collect_left_input(
         bounds = None;
     }
 
+    let build_has_null = !left_values.is_empty() && 
left_values[0].null_count() > 0;

Review Comment:
    This can:
         - Miss a NULL in the subquery and incorrectly return outer rows.
         - Return a logically NULL outer key when the subquery is non-empty.
   
   we should use logical NULL masks on both sides.



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