houqp commented on a change in pull request #822:
URL: https://github.com/apache/arrow-rs/pull/822#discussion_r725692357



##########
File path: arrow/src/compute/kernels/arithmetic.rs
##########
@@ -182,15 +186,17 @@ where
     //      `values` is an iterator with a known size.
     let buffer = unsafe { Buffer::from_trusted_len_iter(values) };
 
-    let data = ArrayData::new(
-        T::DATA_TYPE,
-        left.len(),
-        None,
-        null_bit_buffer,
-        0,
-        vec![buffer],
-        vec![],
-    );
+    let data = unsafe {

Review comment:
       ```suggestion
       // Safety: generic guarantees array data type matches with buffer value 
iterator datatype
       let data = unsafe {
   ```

##########
File path: arrow/src/array/array_primitive.rs
##########
@@ -124,31 +124,35 @@ impl<T: ArrowPrimitiveType> PrimitiveArray<T> {
     /// Creates a PrimitiveArray based on an iterator of values without nulls
     pub fn from_iter_values<I: IntoIterator<Item = T::Native>>(iter: I) -> 
Self {
         let val_buf: Buffer = iter.into_iter().collect();
-        let data = ArrayData::new(
-            T::DATA_TYPE,
-            val_buf.len() / mem::size_of::<<T as 
ArrowPrimitiveType>::Native>(),
-            None,
-            None,
-            0,
-            vec![val_buf],
-            vec![],
-        );
+        let data = unsafe {

Review comment:
       ```suggestion
           // Safety: Buffer from iterator trait is sound, generic guarantees 
array data type matches iterator item type
           let data = unsafe {
   ```

##########
File path: arrow/src/array/array.rs
##########
@@ -414,15 +416,17 @@ pub fn new_null_array(data_type: &DataType, length: 
usize) -> ArrayRef {
                 new_null_sized_array::<IntervalDayTimeType>(data_type, length)
             }
         },
-        DataType::FixedSizeBinary(value_len) => make_array(ArrayData::new(
-            data_type.clone(),
-            length,
-            Some(length),
-            Some(MutableBuffer::new_null(length).into()),
-            0,
-            vec![Buffer::from(vec![0u8; *value_len as usize * length])],
-            vec![],
-        )),
+        DataType::FixedSizeBinary(value_len) => make_array(unsafe {

Review comment:
       ```suggestion
           // Safety: `value_len` from `DataType::FixedSizeBinary` is used as 
data unit length value buffer length calculation
           DataType::FixedSizeBinary(value_len) => make_array(unsafe {
   ```

##########
File path: arrow/src/array/array_boolean.rs
##########
@@ -212,15 +213,17 @@ impl<Ptr: Borrow<Option<bool>>> FromIterator<Ptr> for 
BooleanArray {
             }
         });
 
-        let data = ArrayData::new(
-            DataType::Boolean,
-            data_len,
-            None,
-            Some(null_buf.into()),
-            0,
-            vec![val_buf.into()],
-            vec![],
-        );
+        let data = unsafe {

Review comment:
       ```suggestion
           // Safety: value buffer size is derived directly from iterator size 
hint above
           let data = unsafe {
   ```




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