tustvold commented on code in PR #3115:
URL: https://github.com/apache/arrow-rs/pull/3115#discussion_r1023559246


##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -489,6 +519,43 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
             )
         }
     }
+
+    /// Returns `PrimitiveBuilder` of this primitive array for mutating its 
values if the underlying
+    /// data buffer is not shared by others.
+    pub fn into_builder(self) -> Result<PrimitiveBuilder<T>, Self> {
+        let null_buffer = self
+            .data
+            .null_buffer()
+            .cloned()
+            .and_then(|b| b.into_mutable().ok());

Review Comment:
   I think this needs to be split into the part that clones the null buffer and 
`into_mutable` after data is dropped.
   
   I also think this method should fail if the null buffer cannot be converted, 
where I think it currently just loses the null buffer



##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -489,6 +519,43 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
             )
         }
     }
+
+    /// Returns `PrimitiveBuilder` of this primitive array for mutating its 
values if the underlying
+    /// data buffer is not shared by others.
+    pub fn into_builder(self) -> Result<PrimitiveBuilder<T>, Self> {
+        let null_buffer = self
+            .data
+            .null_buffer()
+            .cloned()
+            .and_then(|b| b.into_mutable().ok());
+
+        let len = self.len();
+        let null_bit_buffer = self.data.null_buffer().cloned();
+
+        let buffer = self.data.buffers()[0]
+            .slice_with_length(0, len * std::mem::size_of::<T::Native>());

Review Comment:
   The reason I suggested using slice_with_length was because the null buffer 
was being sliced. This is no longer the case so we could not do this, 
ultimately until `into_mutable` supports slices this is all moot anyway



##########
arrow-array/src/builder/null_buffer_builder.rs:
##########
@@ -42,6 +42,16 @@ impl NullBufferBuilder {
         }
     }
 
+    /// Creates a new builder from a `MutableBuffer`.
+    pub fn new_from_buffer(buffer: MutableBuffer, len: usize, capacity: usize) 
-> Self {

Review Comment:
   I think this needs to verify that `len` is less than capacity. And I think 
capacity must be `buffer.len() * 8`



##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -397,6 +397,36 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
         unsafe { build_primitive_array(len, buffer, null_count, null_buffer) }
     }
 
+    /// Applies an unary and infallible function to a mutable primitive array.
+    /// Mutable primitive array means that the buffer is not shared with other 
arrays.
+    /// As a result, this mutates the buffer directly without allocating new 
buffer.
+    ///
+    /// # Implementation
+    ///
+    /// This will apply the function for all values, including those on null 
slots.
+    /// This implies that the operation must be infallible for any value of 
the corresponding type
+    /// or this function may panic.
+    /// # Example
+    /// ```rust
+    /// # use arrow_array::{Int32Array, types::Int32Type};
+    /// # fn main() {
+    /// let array = Int32Array::from(vec![Some(5), Some(7), None]);
+    /// let c = array.unary_mut(|x| x * 2 + 1).unwrap();
+    /// assert_eq!(c, Int32Array::from(vec![Some(11), Some(15), None]));
+    /// # }
+    /// ```
+    pub fn unary_mut<F>(self, op: F) -> Result<PrimitiveArray<T>, 
PrimitiveArray<T>>
+    where
+        F: Fn(T::Native) -> T::Native,
+    {
+        let mut builder = self.into_builder()?;
+        builder

Review Comment:
   :heart: 



##########
arrow-array/src/builder/boolean_buffer_builder.rs:
##########
@@ -33,6 +33,10 @@ impl BooleanBufferBuilder {
         Self { buffer, len: 0 }
     }
 
+    pub fn new_from_buffer(buffer: MutableBuffer) -> Self {

Review Comment:
   Did you?



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