mzabaluev commented on code in PR #8686:
URL: https://github.com/apache/arrow-rs/pull/8686#discussion_r2453285766
##########
arrow-row/src/lib.rs:
##########
@@ -908,14 +908,18 @@ impl RowConverter {
/// [RowConverter]. It will panic if any rows are null. Operations on the
returned [Rows] may
/// panic if the data is malformed.
pub fn from_binary(&self, array: BinaryArray) -> Rows {
+ let (offsets, values, nulls) = array.into_parts();
assert_eq!(
- array.null_count(),
+ nulls.map(|n| n.null_count()).unwrap_or(0),
0,
"can't construct Rows instance from array with nulls"
);
+ let offsets = offsets.iter().map(|&i| i.as_usize()).collect();
+ // Try zero-copy, if it does not succeed, fall back to copying the
values.
+ let buffer = values.into_vec().unwrap_or_else(|values|
values.to_vec());
Rows {
- buffer: array.values().to_vec(),
- offsets: array.offsets().iter().map(|&i| i.as_usize()).collect(),
+ buffer,
+ offsets,
Review Comment:
There is a subtle non-stylistic difference: the shadowing of the `offsets`
variable drops the `i32` offsets array before the data buffer is potentially
allocated for a copy, resulting in less heap fragmentation.
--
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]
