jhorstmann commented on a change in pull request #9413:
URL: https://github.com/apache/arrow/pull/9413#discussion_r570120445
##########
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:
Not sure this should substract the offset. The arrays offset is later
uses to index into the logical elements of both null and data buffers, so they
should have the same length (rounded up to full bytes for the null buffer).
Rather the len of the array below should be `len - offset`.
But even better would be if we don't need the offset parameter at all and
avoid all this confusion ;)
----------------------------------------------------------------
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]
