JosephLenton commented on code in PR #2916:
URL: https://github.com/apache/iceberg-rust/pull/2916#discussion_r4474817184


##########
crates/iceberg/src/spec/values/serde.rs:
##########
@@ -106,6 +109,7 @@ pub(crate) mod _serde {
         Double(f64),
         String(String),
         Bytes(ByteBuf),
+        Uuid(#[serde(serialize_with = "apache_avro::serde::fixed::serialize")] 
[u8; 16]),

Review Comment:
   I have removed it and dug into what happens, and come to the conclusion the 
`Uuid` variant is needed.
   
   When using `RawLiteralEnum::Bytes` it to serialised to an 
`apache_avro::types::Value::Bytes`, which is wrong. Avro will not convert that 
to a UUID. That causes errors when you try to insert data, as the conversion is 
wrong. Avro version 0.21.0 _incorrectly_ did allow that. Version 0.22.0 dropped 
it. (That is also why I needed Avro updated first to complete the PR).
   
   The correct conversion is to a an `apache_avro::types::Value::Fixed`. That 
matches the Java behaviour.
   
   To do that requires serialising as a `[u8; 16]`, which is why I added 
`Uuid(#[serde(serialize_with = "apache_avro::serde::fixed::serialize")] [u8; 
16]),`.
   
   I hope that explains the reasoning here. Either this needs to stay, or a 
larger change is needed elsewhere.



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