-
Notifications
You must be signed in to change notification settings - Fork 1.7k
AVRO-4197: Fix schema bytes defaults #3543
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
Conversation
|
@martin-g Would you mind reapproving/retriggering the workflows? |
|
Thanks for this contribution! This was definitely a behaviour change with unexpected consequences, and doesn't exist in Avro 1.11.5. However, I can't seem to figure out what the PR that caused this change was fixing -- @kellen, do you think you could take another look at #2529 and see if reverting it would be a preferable solution to your issue? |
|
Per the original PR "Current JacksonUtils is not symmetric"; I don't know if this was a "real problem" or a theoretical, annoyingly inconsistent behavior. I didn't dig too deeply into it but it seemed like the |
|
Hello, |
|
@clesaec That's the root of the current bug, yes, but I presume the encoding change to bytes was a desired outcome in some cases? Or was it just the result of trying to get a logical round-trip inside of jackson? The side effect of your original change was to change the way schema defaults were encoded, which I would guess was not desired. |
|
Hello @clesaec ! It's nice to hear from you! My apologies for not bringing you in sooner -- I thought you were away. I created #3584 to investigate if we see any regressions in reverting to the 1.11.x behaviour -- it might be a good idea to reopen that JIRA and see if there's anything that needs to be fixed. @kellen Let's make sure this gets fixed in the next minor release -- I'm working on it and it's in the list! Can you take a look at the revert PR and see if that's a solution that works for you? |
|
Hi @RyanSkraba , |
|
@RyanSkraba a revert would resolve the schema defaults issue, yes. |
|
I reverted the change and I'll reopen the JIRA with a comment! Thanks for your contribution and your patience! If you want to reopen, rebase on |
What is the purpose of the change
AVRO-4197 Fixes incorrect handling of
bytesin the context of schema production introduced in #2529 (AVRO-3876).I am ambivalent about needing a special-case parameter
inSchemaContextto revert back to the previous behavior. It seems like the basis for #2529 was that public json values (e.g. in metadata or via avro-tools) would more properly use the base64 encoding now provided rather than the previousISO_8859_1encoding. SinceJacksonUtilsis used in both situations, some kind of use-case specific switch is unfortunately required.AVRO-3843 seems potentially related to this issue but I have not investigated.
Verifying this change
This change added tests and can be verified as follows:
TestScheam.byteArrayDefaultField()test verifying that the defaultbytesvalues can round-tripDocumentation