jorgecarleitao commented on a change in pull request #9413:
URL: https://github.com/apache/arrow/pull/9413#discussion_r570666451



##########
File path: rust/arrow/src/array/data.rs
##########
@@ -467,6 +467,87 @@ impl ArrayData {
     }
 }
 
+#[inline]
+fn debug_check_primitive<T: ArrowNativeType>(
+    values: &Buffer,
+    nulls: &Option<Buffer>,
+    offset: usize,
+) -> usize {
+    debug_assert_eq!(values.len() % std::mem::size_of::<T>(), 0,
+            "The length of the values buffer must be multiple of the size_of 
of the primitive type");
+    debug_assert_eq!(
+        values.as_ptr().align_offset(std::mem::align_of::<T>()),
+        0,
+        "The values `buffer` pointer must be aligned with the primitive type"
+    );
+    let len = values.len() / std::mem::size_of::<T>();
+    debug_assert!(
+        offset <= len,
+        "the offset must be smaller than the buffers' length"
+    );
+    if let Some(ref nulls) = nulls {
+        debug_assert!(nulls.len() == (len - offset).saturating_add(7) / 8);
+    }
+    len
+}
+
+impl ArrayData {
+    /// Creates a new [`ArrayData`] for a primitive type, verifying all 
assumptions for consumers to
+    /// soundly:
+    /// * transmute `values` to `&[T]`
+    /// * read bits from the null bitmap up to `ArrayData::len`.
+    /// `ArrayData::len` will be set to be `values / size_of::<T::Native>`.
+    /// # Panic
+    /// This function panics iff any of the following:
+    /// * `values.len()` is not a multiple of `size_of::<T::Native>`
+    /// * `values.as_ptr()` is not aligned with `T::Native`
+    /// * when `nulls` is `Some`, `nulls.len` is not equal to `(ArrayData::len 
+ 7) / 8`.
+    /// * `offset > ArrayData::len`
+    #[inline]
+    pub fn new_primitive<T: ArrowPrimitiveType>(
+        values: Buffer,
+        nulls: Option<Buffer>,
+        offset: usize,
+    ) -> Self {
+        assert_eq!(values.len() % std::mem::size_of::<T::Native>(), 0,
+            "The length of the values buffer must be multiple of the size_of 
of the primitive type");
+        assert_eq!(
+            values
+                .as_ptr()
+                .align_offset(std::mem::align_of::<T::Native>()),
+            0,
+            "The values `buffer` pointer must be aligned with the primitive 
type"
+        );
+        let len = values.len() / std::mem::size_of::<T::Native>();
+        assert!(
+            offset <= len,
+            "the offset must be smaller than the buffers' length"
+        );
+        if let Some(ref nulls) = nulls {
+            assert!(nulls.len() == (len - offset).saturating_add(7) / 8);

Review comment:
       Good point. I removed the offset. 👍 

##########
File path: rust/arrow/src/array/data.rs
##########
@@ -467,6 +467,87 @@ impl ArrayData {
     }
 }
 
+#[inline]
+fn debug_check_primitive<T: ArrowNativeType>(
+    values: &Buffer,
+    nulls: &Option<Buffer>,
+    offset: usize,
+) -> usize {
+    debug_assert_eq!(values.len() % std::mem::size_of::<T>(), 0,
+            "The length of the values buffer must be multiple of the size_of 
of the primitive type");
+    debug_assert_eq!(
+        values.as_ptr().align_offset(std::mem::align_of::<T>()),
+        0,
+        "The values `buffer` pointer must be aligned with the primitive type"
+    );
+    let len = values.len() / std::mem::size_of::<T>();
+    debug_assert!(
+        offset <= len,
+        "the offset must be smaller than the buffers' length"
+    );
+    if let Some(ref nulls) = nulls {
+        debug_assert!(nulls.len() == (len - offset).saturating_add(7) / 8);
+    }
+    len
+}
+
+impl ArrayData {
+    /// Creates a new [`ArrayData`] for a primitive type, verifying all 
assumptions for consumers to
+    /// soundly:
+    /// * transmute `values` to `&[T]`
+    /// * read bits from the null bitmap up to `ArrayData::len`.
+    /// `ArrayData::len` will be set to be `values / size_of::<T::Native>`.
+    /// # Panic
+    /// This function panics iff any of the following:
+    /// * `values.len()` is not a multiple of `size_of::<T::Native>`
+    /// * `values.as_ptr()` is not aligned with `T::Native`
+    /// * when `nulls` is `Some`, `nulls.len` is not equal to `(ArrayData::len 
+ 7) / 8`.
+    /// * `offset > ArrayData::len`
+    #[inline]
+    pub fn new_primitive<T: ArrowPrimitiveType>(
+        values: Buffer,
+        nulls: Option<Buffer>,
+        offset: usize,
+    ) -> Self {
+        assert_eq!(values.len() % std::mem::size_of::<T::Native>(), 0,
+            "The length of the values buffer must be multiple of the size_of 
of the primitive type");
+        assert_eq!(
+            values
+                .as_ptr()
+                .align_offset(std::mem::align_of::<T::Native>()),
+            0,
+            "The values `buffer` pointer must be aligned with the primitive 
type"
+        );
+        let len = values.len() / std::mem::size_of::<T::Native>();
+        assert!(
+            offset <= len,
+            "the offset must be smaller than the buffers' length"
+        );
+        if let Some(ref nulls) = nulls {
+            assert!(nulls.len() == (len - offset).saturating_add(7) / 8);
+        }
+        ArrayData::new(T::DATA_TYPE, len, None, nulls, offset, vec![values], 
vec![])
+    }
+
+    /// Creates a new [`ArrayData`] for a primitive type.
+    /// `ArrayData::len` will be set to `values / size_of::<T::Native>`.
+    /// # Safety
+    /// The caller must ensure that:
+    /// * `values.len()` is a multiple of `size_of::<T::Native>`
+    /// * `values.as_ptr()` is aligned with `T::Native`
+    /// * when `nulls` is `Some`, `nulls.len` is equal to `(ArrayData::len + 
7) / 8`.
+    /// * `offset <= ArrayData::len`
+    #[inline]
+    pub unsafe fn new_primitive_unchecked<T: ArrowPrimitiveType>(
+        values: Buffer,
+        nulls: Option<Buffer>,
+        offset: usize,

Review comment:
       Good point. I removed the offset. 👍 




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to