Jefffrey commented on code in PR #10640:
URL: https://github.com/apache/arrow-rs/pull/10640#discussion_r3818107588
##########
parquet-variant-compute/src/variant_array_builder.rs:
##########
@@ -458,22 +460,21 @@ impl VariantBuilderExt for
VariantValueArrayBuilderExt<'_> {
}
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");
-
- let mut builder = BinaryViewBuilder::with_capacity(offsets.len());
- let block = builder.append_block(buffer.into());
- // TODO this can be much faster if it creates the views directly during
append
- let mut start = 0;
+ // Each builder records the current buffer length after appending a row,
so offsets are
+ // monotonically increasing and bounded by the final buffer length.
+ assert!(buffer.len() < u32::MAX as usize);
+
+ let buffer = Buffer::from(buffer);
+ let mut views = Vec::with_capacity(offsets.len());
+ let mut start = 0_usize;
for end in offsets {
- let end = end as u32; // Safe cast: validated max offset fits in u32
above
- builder
- .try_append_view(block, start, end - start)
- .expect("Failed to append view");
+ views.push(make_view(&buffer[start..end], 0, start as u32));
Review Comment:
buffer & offsets here is guaranteed to come from trusted/correct data?
--
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]
