LuciferYang commented on code in PR #2906:
URL: https://github.com/apache/iceberg-rust/pull/2906#discussion_r3869189905
##########
crates/iceberg/src/arrow/reader/predicate_visitor.rs:
##########
@@ -258,20 +264,55 @@ impl PredicateConverter<'_> {
}
}
-/// 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. Predicates
can only
+/// bind to primitive leaves in top-level or struct-nested positions (list/map
interiors
+/// have no accessor), so every path segment before the leaf resolves to 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();
+
+ 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 `{part}` but
found {:?}.",
Review Comment:
Fixed in bfb3a2e6. The message now names the segment whose array was not a
struct, and the nested-field lookup failure names the enclosing struct as well.
##########
crates/iceberg/src/arrow/reader/predicate_visitor.rs:
##########
@@ -258,20 +264,55 @@ impl PredicateConverter<'_> {
}
}
-/// 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. Predicates
can only
+/// bind to primitive leaves in top-level or struct-nested positions (list/map
interiors
+/// have no accessor), so every path segment before the leaf resolves to a
struct.
Review Comment:
Reworded in bfb3a2e6. The comment now states that `Schema::build_accessors`
builds accessors only for primitives and primitives nested in structs, and none
for a list or map or anything inside one.
--
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]
