LuciferYang commented on code in PR #2906:
URL: https://github.com/apache/iceberg-rust/pull/2906#discussion_r4131230703
##########
crates/iceberg/src/arrow/reader/predicate_visitor.rs:
##########
@@ -262,20 +270,59 @@ fn constant_bool_array(value: bool, len: usize) ->
BooleanArray {
BooleanArray::new(buffer, None)
}
-/// Gets the leaf column from the record batch for the required column index.
Only
-/// supports top-level columns for now.
+/// Walks the Parquet column path (root to leaf) through the projected record
batch to
+/// reach a primitive leaf. A single-element path returns the matching
top-level column;
+/// a longer path descends through `StructArray` children by name.
`Schema::build_accessors`
+/// builds accessors only for primitives and primitives nested in structs —
never for a list
+/// or map, nor for anything inside one — so `Reference::bind` rejects those
predicates and
+/// every path segment before the leaf here is a struct.
fn project_column(
batch: &RecordBatch,
- column_idx: usize,
+ path: &[String],
) -> std::result::Result<ArrayRef, ArrowError> {
- let column = batch.column(column_idx);
-
- match column.data_type() {
- DataType::Struct(_) => Err(ArrowError::SchemaError(
- "Does not support struct column yet.".to_string(),
- )),
- _ => Ok(column.clone()),
- }
+ let (root_name, rest) = path
+ .split_first()
+ .ok_or_else(|| ArrowError::SchemaError("Predicate column path is
empty.".to_string()))?;
+
+ let mut current = batch
+ .column_by_name(root_name)
+ .ok_or_else(|| {
+ ArrowError::SchemaError(format!(
+ "Predicate column root `{root_name}` not found in projected
record batch."
+ ))
+ })?
+ .clone();
+ let mut current_name = root_name;
+
+ for part in rest {
+ let struct_array = current
+ .as_any()
+ .downcast_ref::<StructArray>()
+ .ok_or_else(|| {
+ ArrowError::SchemaError(format!(
+ "Predicate column path expected a struct at
`{current_name}` but found {:?}.",
+ current.data_type()
+ ))
+ })?;
+ // `flatten` ANDs the struct's validity into each child, so a leaf
under a null
+ // parent struct reads as null (spec: a null parent implies a null
leaf).
+ let (fields, mut columns) = struct_array.flatten();
+ let (idx, _) = fields.find(part).ok_or_else(|| {
+ ArrowError::SchemaError(format!(
+ "Predicate column nested field `{part}` not found in struct
`{current_name}`."
+ ))
+ })?;
+ current = columns.swap_remove(idx);
Review Comment:
Added in 2439ce24f: `test_predicate_on_two_leaves_of_same_struct` reads a
file with `person: optional struct<age: required int, score: optional int>` and
the predicate `person.age > 25 AND person.score < 250`, so the struct is
projected with both children. It keeps `[1, 4, 6]`. I confirmed it fails the
way you described: swapping `swap_remove(idx)` for `swap_remove(0)` makes the
`score` comparison read `age`, the test returns `[1, 3, 4, 6]`, and the
single-leaf tests still pass. Good catch.
--
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]
