Fixes #601: Jackson subtype schema unions are non-deterministic #613
Conversation
beseder1
force-pushed
the
fix/601-deterministic-avro-unions
branch
from
94b2377 to
0fcc23d
Compare
| final List<Schema> unionSchemas = new ArrayList<>(); | ||
| // Initialize with this schema | ||
| if (_type.isConcrete()) { | ||
| unionSchemas.add(_typeSchema); |
There was a problem hiding this comment.
Why not prevent duplicate adds here instead? Check if unionSchemas has exact instance (iterate over, compare with ==), only add if not yet included?
| final Set<Schema> seenSchemas = Collections.newSetFromMap(new IdentityHashMap<>()); | ||
|
|
||
| for(Schema s : schemas) { | ||
| if(!seenSchemas.contains(s)) { |
There was a problem hiding this comment.
If we were to use Set, instead of set.contains() should rather try to add, check response value (true means entry was added, i.e. was not yet included).
cowtowncoder
added
the
cla-needed
|
Good start! PR much appreciated. One process thing: we need CLA (from either https://github.com/FasterXML/jackson/blob/main/contributor-agreement.pdf (individual) or https://github.com/FasterXML/jackson/blob/main/contributor-agreement.pdf (Corporate CLA)). This is only needed once per contributor, one is good for all future contributions. The usual way is to print, fill & sign, email to Looking forward to merging this PR (and backporting in 2.19, I think). |
With the current implementation of the
RecordVisitor, Avro schema unions are handled in a non-deterministic way. This can cause deserialization errors when differentAvroMapperinstances are used for serialization and deserialization.Specifically, because the subtype ordering is unstable, deserialization may attempt to resolve a serialized object against the wrong subtype, resulting in errors.
This PR makes the ordering deterministic by using an
ArrayListinstead of aSet, ensuring consistent behavior across serialization and deserialization.