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

Fix: Union reader schema resolution #358

Merged

Conversation

redaLaanait
Copy link
Contributor

In the Union reader case, we check that the writer schema is compatible with at least one reader schema and resolve the composite schema against the first compatible found one.

We may pass through some incompatible schemas first; we need to explicitly check compatibility for each iteration by calling the compatible function (or Resolve instead of resolve).

The compatible function does cache its results, and the added calls will likely hit the cache, so I'm optimistic about the performance :)

@nrwiersma
Copy link
Member

So I completely understand the issue, We know one of them is a match because of the initial compatible but we are assume the first one as resolve is not returning an error. In an edge case this is in fact the wrong one "chosen" by compatible. Is that correct?

@redaLaanait
Copy link
Contributor Author

That's correct.

Copy link
Member

@nrwiersma nrwiersma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎉

@nrwiersma nrwiersma merged commit 1cb8acd into hamba:main Mar 8, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants