antonsmetanin commented on issue #365: URL: https://github.com/apache/avro-rs/issues/365#issuecomment-3681975843
@Kriskras99 Can you elaborate on why you think it's guess work? I'm not sure I'm following. As far as I understand, the problem with mixed enums is that when `serialize_unit_variant` is called, we don't know whether other variants are units or structs: - if all others are also unit variants, then it must be serialized as Avro enum string; - but if at least one of them is a struct variant, then this one must be serialized as Avro union variant. But as your program demonstrated, for empty struct variants `serde_derive` generates a `serialize_unit_variant` call. That's why I'm saying that true mixed enums should be explicitly prohibited, but enums that use empty struct variants in place of unit ones, which are semantically equivalent, should be supported. And actually the test from my PR (https://github.com/apache/avro-rs/blob/a4a430fc44deca1fcdd1df1b86d03ca6929fd07a/avro/tests/union_serialization.rs) shows that they are working fine without requiring `SchemaAwareSerializer` or adding the `AvroSchema` trait bound to `to_value`. As for deriving schema from Rust type definition, yes, it's a whole other can of worms, but we can probably control that with custom annotations. For example, `#[avro_union(variant_name)]` in: ```rust #[derive(AvroSchema, Serialize, Deserialize)] #[avro_union(variant_name)] enum Foo { One(Bar), Two(Bar), } ``` could indicate that it should use variant name as type name: ```json [ { "type": "record", "name": "One", "fields": [{"name": "a", "type": "string"}] }, { "type": "record", "name": "Two", "fields": [{"name": "a", "type": "string"}] } ] ``` Or `#[avro_union_type(name = "Baz")]` in: ```rust #[derive(AvroSchema, Serialize, Deserialize)] #[avro_union(variant_name)] enum Foo { One(Bar), #[avro_union_type(name = "Baz")] Two(Bar), } ``` could mean that it should use generate (or re-use if it's been previously defined) type `Baz` for this specific variant: ```json [ { "type": "record", "name": "Bar", "fields": [{"name": "a", "type": "string"}] }, { "type": "record", "name": "Baz", "fields": [{"name": "a", "type": "string"}] } ] ``` There are some questions to be considered, though, like how should primitives be handled. Since using variant names for types or specifying the type manually makes no sense for them: ```rust enum Foo { One(Bar), Two(String), } ``` On the other hand, struct variants like the following: ```rust enum Foo { One(Bar), Two { a: String }, } ``` can _only_ use variant name for type because the struct is anonymous. But in general I believe it's a separate issue. That is, it would be perfectly fine if `derive` didn't support such enums, as long as `to_value` supported them, since you can always fall back to manually defining the schema. -- 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]
