apache / apache/avro-rs

Support flattening `Option<T>` into the `UnionSchema` of `T`

Open
#517 3 comments 2 reactions 0 assignees View on GitHub
enhancement
Dominant language
Rust
Stars
130
Forks
62
Avg merge
11h
Merged PRs (30d)
45

Description

Original issues #449 and #458 by @PookieBuns

Currently the `SchemaAware(De)Serializer` implementations only support one representation of `Option`: `["null", T::get_schema()]` (order of `null` and `T` does not matter). This means that the following schema:
```json
[
"null",
"int",
"string"
]
```
can only map to the following enum:
```rust
enum Example {
Null,
Int(i32),
String(String),
}
```

It is requested that the following `Option` would also match this schema:
```rust
enum Example {
Int(i32),
String(String),
}
Option
```

This would require the following changes to the serializer:
1. In `serialize_some` and `serialize_none`: Only require that the current schema is a `Schema::Union` with a `Schema::Null` variant
2. 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` everywhere.
3. 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
4. `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:
1. 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
2. In `with_union` reading the index must be skipped if the index is already set.
3. In `deserialize_enum` the `UnionEnumDeserializer` must be provided with the stored index
4. In `UnionEnumDeserializer::variant_seed` the stored index must be used if available

This would require the `AvroSchemaComponent` implementation for `Option` 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.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by tracing the SchemaAware serializer and deserializer methods named in the issue, then inspect UnionSerializer, UnionEnumDeserializer, with_union, and the AvroSchemaComponent implementation for Option. Verify the existing two-variant Option behavior first. Done means an Option can use a union containing null alongside multiple T variants, with correct variant indexes for both serialization and deserialization.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
data
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.