mbutrovich commented on code in PR #3354:
URL: https://github.com/apache/iceberg-rust/pull/3354#discussion_r4198192031
##########
crates/iceberg/src/spec/values/serde.rs:
##########
@@ -44,6 +46,49 @@ 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.
+ pub fn project_by_name(self, struct_type: &StructType) -> Result<Self,
Error> {
Review Comment:
> I'd at least note the limitation in the doc comment; the writer's
`field-id` is in the JSON schema if we ever want a fallback.
I added the limitation to the [doc comment of
`project_by_name`](https://github.com/apache/iceberg-rust/blob/91486882b29deb0d97560543e0764aa00a2988fd/crates/iceberg/src/spec/values/serde.rs#L50-L58).
A duplicate name can't reach the `HashMap`, because apache-avro rejects a
record with two fields of the same name when it parses the writer schema
([`schema/parser.rs#L574-L577`](https://github.com/apache/avro-rs/blob/ec5721cb0c80dcde56c1049a004f1d785abd88cf/avro/src/schema/parser.rs#L574-L577)).
As I read the spec, a manifest's writer schema and its partition spec come
from the same spec, so a field dropped and re-added in a later spec doesn't
meet the old field inside one manifest. Matching by field ID is the first
follow-up listed in #3262. This PR keeps the current name matching so that
reads don't change.
##########
crates/iceberg/src/spec/values/tests.rs:
##########
@@ -1719,3 +1722,60 @@ fn test_datum_to_decimal_rejects_scale_change() {
.contains("Decimal scale conversion is not supported")
);
}
+
+#[test]
+fn raw_literal_project_by_name_reorders_and_fills_fields() {
Review Comment:
> these two use the bare `raw_literal_project_by_name_*` names
[Renamed](https://github.com/apache/iceberg-rust/blob/91486882b29deb0d97560543e0764aa00a2988fd/crates/iceberg/src/spec/values/tests.rs#L1726-L1762)
both with the `test_` prefix.
--
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]
