rluvaton commented on code in PR #8686:
URL: https://github.com/apache/arrow-rs/pull/8686#discussion_r2453231555
##########
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:
Nit:
you can keep the offsets inline:
```suggestion
offsets: offsets.iter().map(|&i| i.as_usize()).collect(),
```
##########
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"
);
Review Comment:
If you move the `into_parts` after the check you won't need to change the
nulls assertion
```suggestion
assert_eq!(
array.null_count(),
0,
"can't construct Rows instance from array with nulls"
);
let (offsets, values, nulls) = array.into_parts();
```
--
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]