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.

Sign up for GitHub

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

Jump to bottom

AVRO-1848: Fix handling of falsey default values in Ruby #96

Closed
wants to merge 1 commit into from
Closed

AVRO-1848: Fix handling of falsey default values in Ruby #96

wants to merge 1 commit into from

Conversation

theturtle32
Copy link
Contributor

@theturtle32 theturtle32 mentioned this pull request May 19, 2016
Closed
@theturtle32
Copy link
Contributor Author

theturtle32 commented May 19, 2016 •
edited

I also updated the order field with the same approach, since I saw from looking at the code that it could suffer from the same problem.

Nevermind, there are no valid falsey values for that field. Removing that modification.

@busbey
Copy link
Contributor

busbey commented Jul 5, 2016

👍 looks good to me. will merge prior to 1.8.2 RCs.

@rdblue
Copy link
Contributor

rdblue commented Sep 4, 2016

Thanks for fixing this, @theturtle32! I've committed this to master. I added a test for never returning :no_default, but it failed so I fixed io.rb.

Could you please close this PR? I forgot to add a comment to the commit to do it. Thanks!

@asfgit asfgit closed this in 30b5d4d Feb 10, 2017
iemejia pushed a commit that referenced this pull request Jun 11, 2021
@poros
Fix union resolution (#103)
183893e 
See comments in #96 for more info.

I am a bit scared of the performance implications in case of many
union fields with default value null to allow backward compatibility, so
I added an explicit branch in the default resolution to handle null
without a couple more allocations.

The most efficient solution would be to precompute the default value as
part of the schema parsing and store it as the default for the field
instead of its JSON representation, as we are instead doing now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@theturtle32 @busbey @rdblue