alamb commented on code in PR #9189:
URL: https://github.com/apache/arrow-rs/pull/9189#discussion_r2702400748
##########
arrow-array/src/array/run_array.rs:
##########
@@ -223,27 +223,40 @@ impl<R: RunEndIndexType> RunArray<R> {
impl<R: RunEndIndexType> From<ArrayData> for RunArray<R> {
// The method assumes the caller already validated the data using
`ArrayData::validate_data()`
fn from(data: ArrayData) -> Self {
- match data.data_type() {
+ let (data_type, len, _nulls, offset, _buffers, child_data) =
data.into_parts();
+
+ match &data_type {
DataType::RunEndEncoded(_, _) => {}
_ => {
panic!(
- "Invalid data type for RunArray. The data type should be
DataType::RunEndEncoded"
+ "Invalid data type {data_type:?} for RunArray. Should be
DataType::RunEndEncoded"
);
}
}
- // Safety
- // ArrayData is valid
- let child = &data.child_data()[0];
- assert_eq!(child.data_type(), &R::DATA_TYPE, "Incorrect run ends
type");
- let run_ends = unsafe {
- let scalar = child.buffers()[0].clone().into();
- RunEndBuffer::new_unchecked(scalar, data.offset(), data.len())
- };
-
- let values = make_array(data.child_data()[1].clone());
+ let [run_end_child, values_child]: [ArrayData; 2] = child_data
+ .try_into()
+ .expect("RunArray data should have exactly two child arrays");
+
+ // deconstruct the run ends child array
+ let (
+ run_end_data_type,
+ _run_end_len,
+ _run_end_nulls,
+ _run_end_offset,
+ run_end_buffers,
+ _run_end_child_data,
+ ) = run_end_child.into_parts();
+ assert_eq!(run_end_data_type, R::DATA_TYPE, "Incorrect run ends type");
+ let [run_end_buffer]: [arrow_buffer::Buffer; 1] = run_end_buffers
+ .try_into()
+ .expect("Run ends should have exactly one buffer");
+ let scalar = ScalarBuffer::from(run_end_buffer);
+ let run_ends = unsafe { RunEndBuffer::new_unchecked(scalar, offset,
len) };
+
+ let values = make_array(values_child);
Self {
- data_type: data.data_type().clone(),
+ data_type,
Review Comment:
Also avoids a `DataType::drop` which is not likely to make a large
difference but is still something
##########
arrow-array/src/array/run_array.rs:
##########
@@ -223,27 +223,40 @@ impl<R: RunEndIndexType> RunArray<R> {
impl<R: RunEndIndexType> From<ArrayData> for RunArray<R> {
// The method assumes the caller already validated the data using
`ArrayData::validate_data()`
fn from(data: ArrayData) -> Self {
- match data.data_type() {
+ let (data_type, len, _nulls, offset, _buffers, child_data) =
data.into_parts();
+
+ match &data_type {
DataType::RunEndEncoded(_, _) => {}
_ => {
panic!(
- "Invalid data type for RunArray. The data type should be
DataType::RunEndEncoded"
+ "Invalid data type {data_type:?} for RunArray. Should be
DataType::RunEndEncoded"
);
}
}
- // Safety
- // ArrayData is valid
- let child = &data.child_data()[0];
- assert_eq!(child.data_type(), &R::DATA_TYPE, "Incorrect run ends
type");
- let run_ends = unsafe {
- let scalar = child.buffers()[0].clone().into();
- RunEndBuffer::new_unchecked(scalar, data.offset(), data.len())
- };
-
- let values = make_array(data.child_data()[1].clone());
+ let [run_end_child, values_child]: [ArrayData; 2] = child_data
+ .try_into()
+ .expect("RunArray data should have exactly two child arrays");
+
+ // deconstruct the run ends child array
+ let (
+ run_end_data_type,
+ _run_end_len,
+ _run_end_nulls,
+ _run_end_offset,
+ run_end_buffers,
+ _run_end_child_data,
+ ) = run_end_child.into_parts();
+ assert_eq!(run_end_data_type, R::DATA_TYPE, "Incorrect run ends type");
+ let [run_end_buffer]: [arrow_buffer::Buffer; 1] = run_end_buffers
+ .try_into()
+ .expect("Run ends should have exactly one buffer");
+ let scalar = ScalarBuffer::from(run_end_buffer);
+ let run_ends = unsafe { RunEndBuffer::new_unchecked(scalar, offset,
len) };
Review Comment:
note the previous code also uses `unsafe` to create a RunEndBuffer (which is
valid during construction of ArrayData)
##########
arrow-array/src/array/run_array.rs:
##########
@@ -223,27 +223,40 @@ impl<R: RunEndIndexType> RunArray<R> {
impl<R: RunEndIndexType> From<ArrayData> for RunArray<R> {
// The method assumes the caller already validated the data using
`ArrayData::validate_data()`
fn from(data: ArrayData) -> Self {
- match data.data_type() {
+ let (data_type, len, _nulls, offset, _buffers, child_data) =
data.into_parts();
+
+ match &data_type {
DataType::RunEndEncoded(_, _) => {}
_ => {
panic!(
- "Invalid data type for RunArray. The data type should be
DataType::RunEndEncoded"
+ "Invalid data type {data_type:?} for RunArray. Should be
DataType::RunEndEncoded"
);
}
}
- // Safety
- // ArrayData is valid
- let child = &data.child_data()[0];
- assert_eq!(child.data_type(), &R::DATA_TYPE, "Incorrect run ends
type");
- let run_ends = unsafe {
- let scalar = child.buffers()[0].clone().into();
- RunEndBuffer::new_unchecked(scalar, data.offset(), data.len())
- };
-
- let values = make_array(data.child_data()[1].clone());
Review Comment:
This is real clone of `ArrayData` , which allocates a Vec, which is no
longer done by this PR
--
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]
