mbutrovich opened a new pull request, #3354: URL: https://github.com/apache/iceberg-rust/pull/3354
## Which issue does this PR close? - Closes #3063. - Part of #3262 (Task 1). ## What changes are included in this PR? `pyiceberg-core` reads manifests 4x to 5x slower than PyIceberg's Cython reader (#3262), and the profile linked there puts about 82% of the Rust parse inside `apache-avro` 0.21. For each entry, 0.21 decodes the bytes into a tree of `Value`s, cloning every field name into a new `String` ([`decode.rs#L265-L285`](https://github.com/apache/avro-rs/blob/04707999f75278fdea927ee8d2a59de41d8f22a7/avro/src/decode.rs#L265-L285)), rebuilds the tree to resolve it against the reader schema ([`types.rs#L1092-L1152`](https://github.com/apache/avro-rs/blob/04707999f75278fdea927ee8d2a59de41d8f22a7/avro/src/types.rs#L1092-L1152)), and then converts it into the serde struct. 0.22's schema-aware deserializer (apache/avro-rs#512) decodes the bytes into the serde struct in one pass. | entries | `main` (0.21) | this PR | |--:|--:|--:| | 1,000 | 33.4 ms | 5.6 ms | | 10,000 | 334.1 ms | 55.9 ms | | 50,000 | 1670.5 ms | 281.7 ms | These are best-of-7 times for `Manifest::parse_avro` in a release build with `opt-level = 3`, on V2 manifests that PyIceberg wrote with deflate, 12 columns with full stats, and an identity partition. - Upgrade `apache-avro` to 0.22 and port to its API. Written manifests don't change. - Read manifest entries with `Reader::into_deser_iter` through a new `Resolved<T>` adapter in `avro/deserializer.rs`, which applies the schema resolution rules that 0.22's deserializer skips. `RawLiteral::project_by_name` matches partition fields to the spec by name, as resolution did. The manifest list reader is unchanged, because #3224 covers it. - Define each named Avro type once when writing. [`schema_to_avro_schema`](https://github.com/apache/iceberg-rust/blob/ecba0b537a1f78ec8125c1a26f6dfab3f95b509e/crates/iceberg/src/avro/schema.rs#L301-L331) repeats `fixed_{L}` and `decimal_{P}_{S}` definitions, which the [Avro specification](https://avro.apache.org/docs/1.12.0/specification/#names) doesn't allow. Both `apache-avro` versions reject a repeated `fixed_{L}`, so `main` can't write two `fixed[L]` partition fields of the same length. 0.22 also rejects a repeated decimal, such as identity and `truncate` partitions on one decimal column. - Keep reading manifests that iceberg-rust wrote with 0.21 and repeated decimal definitions. 0.22 rejects their header, so the reader rewrites each identical repeat as a reference and logs a warning with the manifest path. - Store `StandardKeyMetadata`'s `file_length` as an Avro `long` explicitly, because 0.22 serializes a `u64` as `fixed`. - Ignore `test_schema_with_array_map` until a release includes the fix for apache/avro-rs#654. It only affects `avro_schema_to_schema`, which has no callers outside tests. ### Behavior changes - An `int` field written as `long`, such as `equality_ids` from PyIceberg before apache/iceberg-python#3842, still reads, but a value outside the `int` range is now an error instead of wrapping. - `read_data_files_from_avro` has no header fallback, so Avro it wrote with 0.21 for a partition type with a repeated decimal no longer reads. Fields are still matched by name rather than by field ID. #3262 tracks that. ## Are these changes tested? Yes, with unit tests, a fixture written by `main`, and a comparison against `main`: - Tests in `spec/manifest/mod.rs` read manifests whose writer schema differs from iceberg-rust's in field order, field set, numeric types, unions, and record names. They pass on `main` and on this PR. - Round-trip tests write and read repeated `decimal` and `fixed` partition types. `testdata/manifests/repeated-decimal-type-definitions.avro`, written by `main`, covers the reader fallback, and unit tests cover the header rewrite. - Parsing PyIceberg manifests with 1,000, 10,000, and 50,000 entries gives identical entries on `main` and on this PR. ## AI Disclosure I wrote this PR with help from Claude but understand and support the core approach, implementation, and test coverage. -- 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]
