friendlymatthew commented on code in PR #18718:
URL: https://github.com/apache/datafusion/pull/18718#discussion_r2528728023


##########
datafusion/common/src/hash_utils.rs:
##########
@@ -329,6 +329,40 @@ where
     Ok(())
 }
 
+#[cfg(not(feature = "force_hash_collisions"))]
+fn hash_union_array(
+    array: &UnionArray,
+    random_state: &RandomState,
+    hashes_buffer: &mut [u64],
+) -> Result<()> {
+    let DataType::Union(union_fields, _mode) = array.data_type() else {
+        unreachable!()
+    };
+
+    let mut child_hashes = vec![None; 128];
+    for (type_id, _field) in union_fields.iter() {
+        let child = array.child(type_id);
+        let mut child_hash_buffer = vec![0; child.len()];
+        create_hashes([child], random_state, &mut child_hash_buffer)?;
+
+        child_hashes[type_id as usize] = Some(child_hash_buffer);
+    }
+
+    #[allow(clippy::needless_range_loop)]
+    for i in 0..array.len() {
+        let type_id = array.type_id(i);
+        let child_offset = array.value_offset(i);
+
+        let child_hash = &child_hashes[type_id as usize]
+            .as_ref()
+            .expect("invalid type_id");
+
+        hashes_buffer[i] = child_hash[child_offset];
+    }

Review Comment:
   I found that clippy's suggestion hurt readability
   
   ```sh
   error: the loop variable `i` is used to index `hashes_buffer`
      --> datafusion/common/src/hash_utils.rs:351:14
       |
   351 |     for i in 0..array.len() {
       |              ^^^^^^^^^^^^^^
       |
       = help: for further information visit 
https://rust-lang.github.io/rust-clippy/rust-1.91.0/index.html#needless_range_loop
       = note: `-D clippy::needless-range-loop` implied by `-D warnings`
       = help: to override `-D warnings` add 
`#[allow(clippy::needless_range_loop)]`
   help: consider using an iterator and enumerate()
       |
   351 -     for i in 0..array.len() {
   351 +     for (i, <item>) in 
hashes_buffer.iter_mut().enumerate().take(array.len()) {
       |
   
   ```



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