scovich commented on code in PR #8193:
URL: https://github.com/apache/arrow-rs/pull/8193#discussion_r2291056832
##########
parquet-variant-compute/src/variant_array_builder.rs:
##########
@@ -338,19 +327,21 @@ impl Drop for VariantArrayVariantBuilder<'_> {
}
}
-fn binary_view_array_from_buffers(
- buffer: Vec<u8>,
- locations: Vec<(usize, usize)>,
-) -> BinaryViewArray {
- let mut builder = BinaryViewBuilder::with_capacity(locations.len());
+fn binary_view_array_from_buffers(buffer: Vec<u8>, offsets: Vec<usize>) ->
BinaryViewArray {
+ // All offsets are less than or equal to the buffer length, so we can
safely cast all offsets
+ // inside the loop below, as long as the buffer length fits in u32.
+ u32::try_from(buffer.len()).expect("buffer length should fit in u32");
Review Comment:
No offset can legally be larger than the buffer's length -- the binary view
would reject it -- and indeed the last offset _is_ the buffer's length. It
seemed simpler to just use that fact, which works even when
`offsets.is_empty()`, rather than have to worry about (and document) the
possibly-empty case:
```rust
if let Some(offset) = offsets.last() {
u32::try_from(offset).expect(...);
}
```
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
