Re: [PR] Add Fixed, Uuid support to arrow-avro [arrow-rs]

["klion26 (via GitHub)" <gi...@apache.org>]

klion26 commented on code in PR #7557:
URL: https://github.com/apache/arrow-rs/pull/7557#discussion_r2151219759


##########
arrow-avro/src/reader/record.rs:
##########
@@ -531,4 +546,49 @@ mod tests {
         assert_eq!(map_arr.len(), 1);
         assert_eq!(map_arr.value_length(0), 0);
     }
+
+    #[test]
+    fn test_fixed_decoding() {

Review Comment:
   Thanks for these tests. Do we need to add a test for the UUID type?



##########
arrow-avro/src/codec.rs:
##########
@@ -192,6 +192,8 @@ pub enum Codec {
     /// Represents Avro fixed type, maps to Arrow's FixedSizeBinary data type
     /// The i32 parameter indicates the fixed binary size
     Fixed(i32),
+    /// Represents Avro Uuid type, a FixedSizeBinary with a length of 16

Review Comment:
   In the [rfc link](https://www.ietf.org/rfc/rfc4122.txt), it says that `A UUID is 128 bits long, and requires no central registration process`, does the length `16` here suffice for that?
   
   for the [uuid crate](https://docs.rs/uuid/latest/uuid/), the length for the example `67e55044-10b1-426f-9247-bb680e5fe0c8` is 36



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org