mbutrovich commented on PR #3354: URL: https://github.com/apache/iceberg-rust/pull/3354#issuecomment-6042414924
Thanks @laskoviymishka. I pushed a V1 version of the field audit and the fix for the second error path, and replied in each thread. Some of these apply to `main` with apache-avro 0.21 as well, so I'd like to keep this PR to the upgrade and track them separately. The details are in each thread. > I'd just drop a `// TODO(#2913)` at the `UuidSchema::String` site and note the read-fixed/write-string asymmetry in the description. I added the [TODO](https://github.com/apache/iceberg-rust/blob/fefcbd6d17a262ce240a6b3ae0ee99164d8f6fed/crates/iceberg/src/avro/schema.rs#L274-L276) and a sentence to "Behavior changes" in the description. > I'd add one small test constructing all three writers so a future schema edit fails in CI, and reword the message to state the invariant. I reworded the [message](https://github.com/apache/iceberg-rust/blob/fefcbd6d17a262ce240a6b3ae0ee99164d8f6fed/crates/iceberg/src/spec/manifest_list/writer.rs#L152-L154). [`test_manifest_list_writer_v1`](https://github.com/apache/iceberg-rust/blob/fefcbd6d17a262ce240a6b3ae0ee99164d8f6fed/crates/iceberg/src/spec/manifest_list/writer.rs#L317-L360), [`test_manifest_list_writer_v2`](https://github.com/apache/iceberg-rust/blob/fefcbd6d17a262ce240a6b3ae0ee99164d8f6fed/crates/iceberg/src/spec/manifest_list/writer.rs#L362-L408), and [`test_manifest_list_writer_v3`](https://github.com/apache/iceberg-rust/blob/fefcbd6d17a262ce240a6b3ae0ee99164d8f6fed/crates/iceberg/src/spec/manifest_list/writer.rs#L410-L458) already construct each writer through this `expect`, so a schema edit that breaks it fails them. > a criterion bench in `crates/iceberg/benches` or a link to the harness numbers would let us watch it. The table in the description comes from the harness in its collapsed "Benchmark harness" section, which runs against `main` and against this PR's head commit. -- 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]
