comphead commented on code in PR #3282:
URL: https://github.com/apache/iceberg-rust/pull/3282#discussion_r4127161074
##########
crates/iceberg/src/arrow/record_batch_projector.rs:
##########
@@ -176,21 +176,32 @@ impl RecordBatchProjector {
}
fn get_column_by_field_index(batch: &[ArrayRef], field_index: &[usize]) ->
Result<ArrayRef> {
+ if let [index] = field_index {
+ return Ok(batch[*index].clone());
+ }
+
let mut rev_iterator = field_index.iter().rev();
let mut array = batch[*rev_iterator.next().unwrap()].clone();
- let mut null_buffer = array.logical_nulls();
+ let mut parent_null_buffer = None;
for idx in rev_iterator {
- array = array
+ let struct_array = array
.as_any()
.downcast_ref::<StructArray>()
.ok_or(Error::new(
ErrorKind::Unexpected,
"Cannot convert Array to StructArray",
- ))?
- .column(*idx)
- .clone();
- null_buffer = NullBuffer::union(null_buffer.as_ref(),
array.logical_nulls().as_ref());
+ ))?;
+ parent_null_buffer = NullBuffer::union(
+ parent_null_buffer.as_ref(),
+ struct_array.logical_nulls().as_ref(),
+ );
+ array = struct_array.column(*idx).clone();
}
+ let Some(parent_null_buffer) = parent_null_buffer else {
+ return Ok(array);
+ };
Review Comment:
Consider also skipping the rebuild when the leaf already carries these
nulls. arrow-rs's Parquet reader derives child validity from definition levels,
so nullable children read from Parquet already contain every ancestor null. A
round trip confirmed this for `Int32` and `Utf8` leaves. Required leaves come
back without a null buffer and still take the rebuild.
With the changes above plus a `contains` check, the parent-nulls path
measured 259 ns → 117 ns for `Int32` and 18.3 µs → 106 ns for a 240 KiB `Utf8`
leaf. A failed check costs about 20 ns.
A short comment may help here too, since `NullBuffer::union` returning
`None` for all-valid masks is what keeps sliced batches on the zero-copy path.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
