feat: Support (De)Serialization for different representations of Nullable Unions by PookieBuns · Pull Request #458 · apache/avro-rs

PookieBuns · 2026-02-10T09:27:05Z

Fixes The Rust Representation Conundrum as mentioned in #449, which allows (de)serializing, all avro-rs, rusty, and avro_json_encoding_compatible rust representations to avro. Tested using the provided test suite in the mentioned issue.

Note: This does not address #365 since this only applies to schema aware serialization.

I considered using enum variant name as lookup method for picking Union branch when serializing, shown here, but given the current state in which Value doesn't hold the name for named types, I realized it's not possible to do the same for deserialization until further work to support names in Value has been done.

Therefore, this implementation makes the assumption that if you are using Option<T> to represent a nullable union, that means that the null is the first variant in the union and serialization / deserialization just bump / decrement the index lookup respectively.

I believe this doesn't actually produce any breaking changes, because the status quo is that tagged Option "Rusty" representations straight up don't work.

Please let me know if you have any questions!

avro/src/serde/de.rs

martin-g · 2026-02-10T14:09:44Z

avro/src/serde/de.rs

+                {
+                    return visitor.visit_enum(EnumUnitDeserializer::new(field));
+                }
+                // Assume `null` is the first branch if deserializing some so decrement the variant index


This assumption is not guaranteed by the spec :-/
We should add some index/length checks to avoid panics/overflows.

Updated to use checked_sub to ensure underflow doesn't happen.

Regarding the topic about not being enforced by spec, even though it isn't strictly enforced, it's highly recommended https://avro.apache.org/docs/1.11.1/specification/#unions

Note that when a default value is specified for a record field whose type is a union, the type of the default value must match the first element of the union. Thus, for unions containing “null”, the “null” is usually listed first, since the default value of such unions is typically null.

For serialization we technically could get around this by resolving union branches using variant name, but for deserialization cannot yet because it's not schema aware and/or Value doesn't hold name information, which is why I chose to use index bumping on serialization too for consistent behavior. This seems like the closest we can get without a significant refactor or feature for deserialization.

For now, the behavior will intentionally throw an error if you try to (de)serialize an Option with null not being the first branch. This doesn't break existing behavior because the status quo is non-functional so although not a perfect solution imo it's a step better than what we have now

martin-g · 2026-02-10T14:10:42Z

avro/src/serde/ser_schema/mod.rs

+                        }
+                    }
+                }
+                let branch_index = variant_index as usize + usize::from(self.serializing_some);


Again, this assumes that the null is the first variant in the union.

Updated to throw error if null isn't first branch, reasoning stated in #458 (comment)

martin-g · 2026-02-10T14:12:05Z

avro/tests/nullable_union.rs

+        "name": "MyUnion",
+        "type": [
+            "null",
+            "int"


Please add a test where the null is not the first variant too.

Added tests in 6f691db to ensure null as non-first variant when using Option fails with error message communicating intended behavior

martin-g · 2026-02-10T14:13:06Z

avro/src/serde/ser_schema.rs

+                if branch_index >= union_schema.schemas.len() {
+                    return Err(create_error(format!(
+                        "Variant index out of bounds: {}. The union schema has '{}' schemas",
+                        variant_index,


Suggested change

variant_index,

branch_index,

martin-g · 2026-02-10T14:13:52Z

avro/src/serde/ser_schema.rs

+                if branch_index >= union_schema.schemas.len() {
+                    return Err(create_error(format!(
+                        "Variant index out of bounds: {}. The union schema has '{}' schemas",
+                        variant_index,


Suggested change

variant_index,

branch_index,

martin-g · 2026-02-10T14:14:54Z

avro/src/serde/ser_schema/mod.rs

+                                }
+                                encode_int(i as i32, &mut self.writer)?;
+                                return encode_int(variant_index as i32, &mut self.writer);
+                            }


This should probably support Schema::Ref too, to refer to Schema::Enum

updated in 6c58e62 with test to verify behavior

avro/src/serde/de.rs

avro/src/serde/ser_schema.rs

martin-g · 2026-02-10T14:16:52Z

avro/tests/nullable_union.rs

+    }
+
+    #[test]
+    #[ignore]


Please add a comment why it is ignored

Updated to instead expect a failure / comment on why it will fail. I'm also open to just removing it if you think its out of scope, since that mod is mostly to demonstrate how untagged enums have edge cases that fail and check that tagged enums do not

PookieBuns · 2026-02-11T03:49:13Z

avro/src/serde/de.rs

+                let variant_idx = (*idx as usize)
+                    .checked_sub(usize::from(self.deserializing_some))
+                    .ok_or_else(|| Details::GetNull(inner.deref().clone()))?;


Do we want to add an Error variant here to communicate more clearly that deserializing some when null isn't first isn't supported? Right now it just tells the user Expects null for 0th when deserializing some

PookieBuns · 2026-02-11T13:25:03Z

Updated to fix clippy / license checks cb749c3

representations

Co-authored-by: Martin Grigorov <martin-g@users.noreply.github.com>

PookieBuns · 2026-03-08T10:35:29Z

Updated to use new methods introduced by previous commits

Kriskras99 · 2026-03-21T10:58:53Z

Hi @PookieBuns, sorry for not responding earlier. I've been working on #512 and was really hoping to get it finished earlier. It has quite an impact on what you're trying to achieve in this PR.

I've adapted your tests to run on #512 (nullable_union.rs). Note that I've combined the serialize and deserialize tests into a roundtrip test.

There are two kinds of tests that fail on my PR. The first kind has expects a Option to flatten to the underlying union. These tests are marked as should_panic in the provided gist as I've explicitly chosen not to support this at this point as it adds extra complexity in the serializer/deserializer and derive code.

The second kind are the ones with JSON enums. I think I finally understand what you're trying to achieve, but please correct me if I'm wrong. You are (de)serializing Avro encoded as JSON through serde_json into types. Then you also want to use those same types to (de)serialized Avro encoded as binary. I had originally assumed you were using the TryFrom<serde_json::Value> for Value and TryFrom<Value> for serde_json::Value> implementations. We cannot support your usecase of using the same types for binary Avro and JSON Avro (via serde_json). What is an option is to provide proper support for JSON Avro in this crate. That would be through something like from_json<T: Deserialize>(json: serde_json::Value, schema: &Schema> and to_json<T: Serialize>(value: &T, schema: &Schema) (don't take this as gospel, there are some API considerations to make and you need to deal with schemata).

Please let me know if I understand your usecase correctly and if the provided solution would be helpful. Sorry for all the work you put into this PR, but I don't think we can merge this.

PookieBuns · 2026-03-25T00:05:13Z

Hi @Kriskras99 thanks for the response. Being a maintainer is hard work and I appreciate all that you've done for the rust community.

In response to q1, does this mean that apache-avro only plans on supporting

[
  "null",
  "int",
  {
    "type": "enum",
    "name": "MyEnum",
    "symbols": [
      "A",
      "B"
    ]
  },
  {
    "type": "record",
    "name": "MyRecord",
    "fields": [
      {
        "name": "a",
        "type": "int"
      }
    ]
  }
]

via

    enum MyEnum {
        A,
        B,
    }

    struct MyRecord {
        a: i32,
    }

    enum MyUnionNullable {
        Null,
        Int(i32),
        MyEnum(MyEnum),
        MyRecord(MyRecord),
    }

and not via

    enum MyEnum {
        A,
        B,
    }

    struct MyRecord {
        a: i32,
    }

    enum MyUnion {
        Int(i32),
        MyEnum(MyEnum),
        MyRecord(MyRecord),
    }

    Option<MyUnion>

? IMO intuitively, the latter way is the more idiomatic way a common user would expect to define a struct that matches the schema defined above, and I believe is also how https://github.com/lerouxrgd/rsgen-avro currently generates its structs (albeit they do some custom implementation of serde_serialize). The advantage of this representation is nicer integration with Option semantics instead of having to deal with Null branches manually for end users

For point 2, Yea I agree with you fully, converting from avro value to json value should be its own implementation instead of using serde json. I just so happened to be able to fix it with my implementation which is why I had it included, but I would be completely fine if it wasn't supported. This use case doesn't really matter for me.

I really hope we could support the "rust idiomatic" representation for both serialization and deserialization referred to in point 1, but ultimately you are the maintainer (or core contributer) and I respect your decision. If you would like any help with implementation. Feel free to reach out to me! Thanks!

Kriskras99 · 2026-03-25T13:58:43Z

Closed in favor of #512 and #517

martin-g reviewed Feb 10, 2026

View reviewed changes

PookieBuns commented Feb 11, 2026

View reviewed changes

PookieBuns requested a review from martin-g February 11, 2026 13:25

Kriskras99 mentioned this pull request Feb 19, 2026

Release 0.22 ?! #424

Open

allenyuchen and others added 13 commits March 8, 2026 03:16

test: Add tests for nullable union

6e098c0

feat: Support deserializing tagged enums

a52f495

feat: Support serializing tagged enums

815660f

feat: Support serializing / deserializing all 3 rust union rust

0bd1e77

representations

test: Update tests to match repo structure

36e514b

chore: Clearer error messages for deserialization

f9c5e3a

Co-authored-by: Martin Grigorov <martin-g@users.noreply.github.com>

test: Add comment on why untagged will fail in untagged_pitfall

388ff89

feat: Intentionally throw an error for unions with non first null

10c0005

chore: Fix error message to use branch index

c08b12a

chore: Support ref schema when serializing Option<Enum>

59ddbcb

chore: Make new_deserializing_some private

9d8014b

Co-authored-by: Martin Grigorov <martin-g@users.noreply.github.com>

chore: Make new_serializing_some private

77f7020

Co-authored-by: Martin Grigorov <martin-g@users.noreply.github.com>

chore: Fix clippy and license checks

0eff6f4

PookieBuns force-pushed the main branch from cb749c3 to 0eff6f4 Compare March 8, 2026 10:16

chore: Use new name() and builder() methods

3d18277

Kriskras99 mentioned this pull request Mar 25, 2026

Support flattening Option<T> into the UnionSchema of T #517

Open

Kriskras99 closed this Mar 25, 2026

Conversation

PookieBuns commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PookieBuns Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PookieBuns Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PookieBuns Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PookieBuns commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PookieBuns commented Mar 8, 2026

Uh oh!

Kriskras99 commented Mar 21, 2026

Uh oh!

PookieBuns commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kriskras99 commented Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

PookieBuns commented Feb 10, 2026 •

edited

Loading

PookieBuns Feb 10, 2026 •

edited

Loading

PookieBuns Feb 10, 2026 •

edited

Loading

PookieBuns Feb 10, 2026 •

edited

Loading

PookieBuns commented Feb 11, 2026 •

edited

Loading

PookieBuns commented Mar 25, 2026 •

edited

Loading