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


##########
datafusion/physical-plan/src/joins/join_hash_map.rs:
##########
@@ -569,4 +590,70 @@ mod tests {
         assert_eq!(input_indices, vec![1, 1]);
         assert_eq!(match_indices, vec![3, 1]);
     }
+
+    #[test]
+    fn test_unique_fast_path_with_uninserted_row() {
+        // Row 1 is left out, as `update_hash` does for a NULL key under
+        // `NullEquality::NullEqualsNothing`. Every hash value still maps to a
+        // single row, so the probe takes the unique-key fast path.
+        let build_hashes = [10u64, 99, 20, 30];
+        let mut hash_map = JoinHashMapU32::with_capacity(build_hashes.len());
+        hash_map.update_from_iter(
+            Box::new(build_hashes.iter().enumerate().filter(|(row, _)| *row != 
1)),
+            0,
+        );
+
+        let probe_hashes = vec![10, 20, 30];
+        let mut input_indices = vec![];
+        let mut match_indices = vec![];
+        let next_offset = hash_map.get_matched_indices_with_limit_offset(
+            &probe_hashes,
+            None,
+            2,
+            (0, None),
+            &mut input_indices,
+            &mut match_indices,
+        );
+        // The fast path limits by probe rows and resumes at probe row 2; the
+        // chain walk would stop after two matches at `(1, Some(0))`.
+        assert_eq!(next_offset, Some((2, None)));
+        assert_eq!(input_indices, vec![0, 1]);
+        assert_eq!(match_indices, vec![0, 2]);
+
+        let next_offset = hash_map.get_matched_indices_with_limit_offset(
+            &probe_hashes,
+            None,
+            2,
+            (2, None),
+            &mut input_indices,
+            &mut match_indices,
+        );
+        assert_eq!(next_offset, None);
+        assert_eq!(input_indices, vec![2]);
+        assert_eq!(match_indices, vec![3]);
+    }
+
+    #[test]
+    fn test_chain_from_earlier_update_disables_fast_path() {

Review Comment:
   Optional follow-up: could you add a test where a chain is first created 
across separate updates? For example, insert hash 10 at row 0, skip row 1, then 
insert hash 10 at row 2 in a later update, and probe with limit 1 while 
resuming until both build rows appear exactly once. It would be useful to cover 
this for both U32 and U64 maps.



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