badalprasadsingh commented on code in PR #524: URL: https://github.com/apache/iceberg-go/pull/524#discussion_r2323965318
########## manifest.go: ########## @@ -37,6 +37,19 @@ import ( "github.com/hamba/avro/v2/ocf" ) +func init() { + avro.Register("fixed", [1]byte{}) + avro.Register("fixed", [2]byte{}) + avro.Register("fixed", [3]byte{}) + avro.Register("fixed", [4]byte{}) + avro.Register("fixed", [5]byte{}) + avro.Register("fixed", [6]byte{}) + avro.Register("fixed", [7]byte{}) + avro.Register("fixed", [8]byte{}) + avro.Register("fixed", [16]byte{}) Review Comment: Yes, you are right but, when we have a `nullable` fixed schema like: ```json ["null",{"name":"fixed","type":"fixed","size":16}] ``` The hamba/avro library needs to know how to map Go types to the Avro schema types. For fixed schemas, it expects specific byte array types like [16]byte, but the library's type resolution system couldn't automatically handle all the different fixed sizes. Doing `avro.Register`, says that when it sees a [N]byte type, treat it as a 'fixed' schema type for `union` resolution purposes. One possible fix for it is for fixed type use: ```go internal.NullableSchema(internal.BinarySchema) ``` instead of ```go fixedSchema := internal.Must(avro.NewFixedSchema("fixed", "", typ.len, nil)) sc = internal.NullableSchema(fixedSchema) ``` This would help us get rid of `avro.Register` cases. -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org