Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Union codecs do not match by namespace #385

Open
cryptism opened this issue Oct 15, 2021 · 3 comments
Open

Union codecs do not match by namespace #385

cryptism opened this issue Oct 15, 2021 · 3 comments
Labels
bug Something isn't working

Comments

@cryptism
Copy link

Given two events both called MyEvent that are records with the namespaces org.foo.events and org.bar.events, the expected behaviour would be to successfully decode a message matching org.bar.events if the respective codec was present in the union. However it fails as altMatching only seems picks up the first matching event in the union chain, which in this case would be the codec for org.foo.events.

Just want to check if this is indeed unexpected behaviour in vulcan, and what the behaviour should be - should both codecs be tested given the event or can altMatching be modified to match on the canonical name of the schema?

Happy to explain more if I haven't been clear here.

@vlovgr
Copy link
Contributor

vlovgr commented Oct 17, 2021

We used to resolve on full name but it was changed in #132 (#136) -- maybe @bplommer can explain more?

@bplommer bplommer added the bug Something isn't working label Oct 17, 2021
@bplommer
Copy link
Member

Yes, I think the current behaviour is wrong. It's really an artifact of how we build on top of the Apache Avro java SDK, which loses information during decoding. What happens is that Avro unions themselves are tagged (they begin with a number field indicating which union member they contain) but the Avro SDK converts them to an untagged union, so we have to use various heuristics to work out which union member we have decoded.

Not matching by namespace is, at least superficially, correct, as name matches only require the unqualified name, not the fullname, to match. However I think we should try to match on fullname first and then fall back to unqualified name if that fails - certainly we should try all alternatives with the same unqualified name before raising an error.

@bplommer
Copy link
Member

bplommer commented May 5, 2022

#445 should allow addressing this properly, since we'll be decoding from a tagged rather than untagged union.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

3 participants