flix-tech / avro-serde-php

Avro Serialisation/Deserialisation (SerDe) library for PHP 7.3+ & 8.0 with a Symfony Serializer integration
https://www.flix.tech/
MIT License
60 stars 36 forks source link

Unions of complex types are not handled correctly #77

Open ulrikestampa opened 1 year ago

ulrikestampa commented 1 year ago

Since the supposedly bugfix https://github.com/flix-tech/avro-serde-php/issues/66 in avro-php 4.3.0 the handling of unions with complex types does not work correctly any more: No matter which of the union members is provided, always the first member is written (with value NULL). For illustration please see the attached file containing tests for a sample schema. UnionsWithoutDefaultsTest.txt

In my opinion, there are multiple aspects causing this problem:

I think a solution would include the following fixes: => A record that is missing an expected field must not pass as a valid record if the field is not nullable. => If an expected field is missing, this field must not be written at all (instead of writing it with value NULL). If the field is not nullable, this must raise an error.

BTW according to Avro spec, default values are not to be used when writing. They only provide a means for the reader to replace missing fields by default values, see https://avro.apache.org/docs/1.10.2/spec.html: default: A default value for this field, only used when reading instances that lack the field for schema evolution purposes. The presence of a default value does not make the field optional at encoding time.

tPl0ch commented 1 year ago

Big thanks for the thorough and detailed report, always helps a lot. I acknowledge this, but we need to prioritize this first. Please give us some time and expect an answer sometime next week.

ulrikestampa commented 1 year ago

Did you already have time to do a first evaluation and estimation? It would be great if you could let us know if/when this probably gets fixed. This would make it easier for us to decide on an interim-solution on our side. Thanks.

tPl0ch commented 8 months ago

@ulrikestampa I am sorry to disappoint, but most of the teams at our organisation are moving away from PHP as primary programming languages, so this library is currently in a limbo state with no real ownership. I am also not sure if and when any work on this will be done. I wish I would have better news - even though it comes half a year late.

ulrikestampa commented 8 months ago

@tPl0ch Thanks for letting us know. Then we will go in search for other solutions.