alamb commented on code in PR #6396:
URL: https://github.com/apache/arrow-rs/pull/6396#discussion_r1764844394


##########
arrow-arith/src/arity.rs:
##########
@@ -557,6 +577,24 @@ mod tests {
         assert_eq!(c, expected);
     }
 
+    #[test]
+    fn test_binary_mut_null_buffer() {
+        let a = Int32Array::from(vec![Some(3), Some(4), Some(5), Some(6), 
None]);
+
+        let b = Int32Array::from(vec![Some(10), Some(11), Some(12), Some(13), 
Some(14)]);
+
+        let r1 = binary_mut(a, &b, |a, b| a + b).unwrap();
+
+        let a = Int32Array::from(vec![Some(3), Some(4), Some(5), Some(6), 
None]);
+        let b = Int32Array::new(
+            vec![10, 11, 12, 13, 14].into(),
+            Some(vec![true, true, true, true, true].into()),
+        );
+
+        let r2 = binary_mut(a, &b, |a, b| a + b).unwrap();

Review Comment:
   ```suggestion
           // unwrap here means that no copying occured
           let r2 = binary_mut(a, &b, |a, b| a + b).unwrap();
   ```



##########
arrow-arith/src/arity.rs:
##########
@@ -313,7 +313,7 @@ where
         ))));
     }
 
-    let nulls = NullBuffer::union(a.logical_nulls().as_ref(), 
b.logical_nulls().as_ref());

Review Comment:
   It still feels to me that we could remove the copy on try_binary_mut, but 
this seems like an improvement to me 🚀 



##########
arrow-arith/src/arity.rs:
##########
@@ -587,6 +625,24 @@ mod tests {
         .expect_err("should got error");
     }
 
+    #[test]
+    fn test_try_binary_mut_null_buffer() {
+        let a = Int32Array::from(vec![Some(3), Some(4), Some(5), Some(6), 
None]);
+
+        let b = Int32Array::from(vec![Some(10), Some(11), Some(12), Some(13), 
Some(14)]);
+
+        let r1 = try_binary_mut(a, &b, |a, b| Ok(a + b)).unwrap();
+
+        let a = Int32Array::from(vec![Some(3), Some(4), Some(5), Some(6), 
None]);
+        let b = Int32Array::new(
+            vec![10, 11, 12, 13, 14].into(),
+            Some(vec![true, true, true, true, true].into()),
+        );
+
+        let r2 = try_binary_mut(a, &b, |a, b| Ok(a + b)).unwrap();

Review Comment:
   ```suggestion
           // unwrap here means that no copying occured
           let r2 = try_binary_mut(a, &b, |a, b| Ok(a + b)).unwrap();
   ```



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

Reply via email to