mbutrovich commented on code in PR #3354:
URL: https://github.com/apache/iceberg-rust/pull/3354#discussion_r4198150067


##########
Cargo.toml:
##########
@@ -46,7 +46,7 @@ unused_qualifications = "deny"
 [workspace.dependencies]
 aes-gcm = "0.10"
 anyhow = "1.0.72"
-apache-avro = { version = "0.21", features = ["snappy", "zstandard"] }
+apache-avro = { version = "0.22", features = ["snappy", "zstandard"] }

Review Comment:
   > The deserializer's union handling rides on how 0.22's `deserialize_any` 
dispatches unions, which isn't a documented public contract
   
   As I read serde's `Deserializer` contract, `deserialize_any` on a 
self-describing format dispatches on the data. 0.22's schema-aware 
`deserialize_any` reads a union's branch index and continues with that branch's 
schema 
([`deser_schema/mod.rs#L228-L272`](https://github.com/apache/avro-rs/blob/ec5721cb0c80dcde56c1049a004f1d785abd88cf/avro/src/serde/deser_schema/mod.rs#L228-L272)).
 The new tests in 
[`avro/deserializer.rs`](https://github.com/apache/iceberg-rust/blob/91486882b29deb0d97560543e0764aa00a2988fd/crates/iceberg/src/avro/deserializer.rs#L357-L456)
 read union and non-union writer values into plain and `Option` targets. A 
`0.22.x` release that changes the dispatch fails them, along with the two union 
tests you named. Would those tests work as the guard instead of a comment in 
`Cargo.toml`?



-- 
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]

Reply via email to