blackmwk commented on code in PR #2773:
URL: https://github.com/apache/iceberg-rust/pull/2773#discussion_r3859136124
##########
crates/iceberg/src/arrow/reader/projection.rs:
##########
@@ -460,13 +442,20 @@ pub(super) fn add_fallback_field_ids_to_arrow_schema(
"Schema already has field IDs"
);
+ let omitted_field_ids: HashSet<i32> = iceberg_schema
+ .as_struct()
+ .fields()
+ .iter()
+ .filter(|field| !type_has_parquet_physical_field(&field.field_type))
+ .map(|field| field.id)
+ .collect();
+ let mut fallback_field_ids = (1_i32..).filter(|field_id|
!omitted_field_ids.contains(field_id));
Review Comment:
This only skips unknown IDs still present in the current schema. For an
ID-less file written while `unknown` field ID 3 existed, after that field is
dropped the physical fields `1, 2, 4` receive fallback IDs `1, 2, 3`; field 4
is then treated as absent and read as null/default. Please preserve the
historical omitted IDs for this fallback path, or reject the ambiguous
fallback, and add a dropped-unknown regression test.
--
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]