Original issues #449 and #458 by @PookieBuns
Currently the SchemaAware(De)Serializer implementations only support one representation of Option<T>: ["null", T::get_schema()] (order of null and T does not matter). This means that the following schema:
[
"null",
"int",
"string"
]
can only map to the following enum:
enum Example {
Null,
Int(i32),
String(String),
}
It is requested that the following Option would also match this schema:
enum Example {
Int(i32),
String(String),
}
Option<Example>
This would require the following changes to the serializer:
- In
serialize_some and serialize_none: Only require that the current schema is a Schema::Union with a Schema::Null variant
- In
serialize_some there are two cases to keep in mind:
- If there are only two variants, serialize using the current implementation.
- If there are more variants, serialize
T using the current union schema but provide a flag to the serializer that the null variant cannot be used. It would be preferable to modify the union schema/create a new union schema so that the null variant does not exist, but as we need a reference to the schema that would mean using a Cow<Schema> everywhere.
- In
get_resolved_union_variant the logic needs to be modified to correct the index:
- If the index is before the
null variant, it can be used as is
- If the index is at or after the
null variant, the index needs to be incremented by one
UnionSerializer would also need to be changed to ignore the null variant when the flag is set
This would require the following changes to the deserializer:
- In
deserialize_option:
- Only require that the current schema is a
Schema::Union with a Schema::Null variant
- If a
null is read, then the current logic can be used
- Otherwise, store the index that was read and
visit_some with the current deserializer
- In
with_union reading the index must be skipped if the index is already set.
- In
deserialize_enum the UnionEnumDeserializer must be provided with the stored index
- In
UnionEnumDeserializer::variant_seed the stored index must be used if available
This would require the AvroSchemaComponent implementation for Option<T> to only panic if T::get_schema is a union with a Schema::Null variant, otherwise modify that schema to include Schema::Null.
There is probably more changes needed, but this is what I can think of right now.
Original issues #449 and #458 by @PookieBuns
Currently the
SchemaAware(De)Serializerimplementations only support one representation ofOption<T>:["null", T::get_schema()](order ofnullandTdoes not matter). This means that the following schema:[ "null", "int", "string" ]can only map to the following enum:
It is requested that the following
Optionwould also match this schema:This would require the following changes to the serializer:
serialize_someandserialize_none: Only require that the current schema is aSchema::Unionwith aSchema::Nullvariantserialize_somethere are two cases to keep in mind:Tusing the current union schema but provide a flag to the serializer that thenullvariant cannot be used. It would be preferable to modify the union schema/create a new union schema so that thenullvariant does not exist, but as we need a reference to the schema that would mean using aCow<Schema>everywhere.get_resolved_union_variantthe logic needs to be modified to correct the index:nullvariant, it can be used as isnullvariant, the index needs to be incremented by oneUnionSerializerwould also need to be changed to ignore the null variant when the flag is setThis would require the following changes to the deserializer:
deserialize_option:Schema::Unionwith aSchema::Nullvariantnullis read, then the current logic can be usedvisit_somewith the current deserializerwith_unionreading the index must be skipped if the index is already set.deserialize_enumtheUnionEnumDeserializermust be provided with the stored indexUnionEnumDeserializer::variant_seedthe stored index must be used if availableThis would require the
AvroSchemaComponentimplementation forOption<T>to only panic ifT::get_schemais a union with aSchema::Nullvariant, otherwise modify that schema to includeSchema::Null.There is probably more changes needed, but this is what I can think of right now.