mbutrovich commented on code in PR #3354:
URL: https://github.com/apache/iceberg-rust/pull/3354#discussion_r4209487082
##########
crates/iceberg/src/spec/values/serde.rs:
##########
@@ -44,6 +46,53 @@ pub(crate) mod _serde {
pub fn try_into(self, ty: &Type) -> Result<Option<Literal>, Error> {
self.0.try_into(ty)
}
+
+ /// Matches the fields of a record to `struct_type` by name, the way
Avro
+ /// schema resolution matches record fields. The result has
+ /// `struct_type`'s fields in its order, with null for an optional
field the
+ /// record lacks. Record fields that `struct_type` lacks are dropped.
Values
+ /// other than records are returned unchanged.
+ ///
+ /// Fields aren't matched by field ID. A writer that stores a field
under
+ /// another name, such as Java replacing characters that Avro names
don't
+ /// allow, reads as a missing field.
+ pub fn project_by_name(self, struct_type: &StructType) -> Result<Self,
Error> {
Review Comment:
> if the writer's names don't line up at all
`main` reads these the same way. I wrote a V2 manifest with an identity
partition on the nested column `a.b` and named the Avro field `a_x2Eb`, as
Java's
[`TypeToSchema`](https://github.com/apache/iceberg/blob/5e7169168db3d34e29354c6f59ec4d6e420b8d2d/core/src/main/java/org/apache/iceberg/avro/TypeToSchema.java#L116-L128)
does. `main` and this PR both read the partition value as null. With the
spec's field also named `a_x2Eb`, `main` reads the value. On `main`, the reader
schema gives each optional partition field a null default
([`schema.rs#L88-L94`](https://github.com/apache/iceberg-rust/blob/ab047b5e2f6e2ba6b999731653a3f7722b84e6f7/crates/iceberg/src/avro/schema.rs#L88-L94)),
and 0.21 [uses that
default](https://github.com/apache/avro-rs/blob/04707999f75278fdea927ee8d2a59de41d8f22a7/avro/src/types.rs#L1113-L1131)
when the writer has no field with that name.
Java writes each partition field's `field-id`, so matching by field ID fixes
this. It's the first follow-up in #3262, which also covers the read side of
#2536. This PR keeps the current name matching, so would it work for you to
handle this there?
--
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]
