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]