Skip to content
New issue

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

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

Already on GitHub? # to your account

Issue decoding fixed decimal field into *big.Rat #424

Closed
cdrexle opened this issue Jul 31, 2024 · 6 comments · Fixed by #425
Closed

Issue decoding fixed decimal field into *big.Rat #424

cdrexle opened this issue Jul 31, 2024 · 6 comments · Fixed by #425

Comments

@cdrexle
Copy link

cdrexle commented Jul 31, 2024

When encoding a fixed decimal field, the source data type is required to be *big.Rat (see https://github.com/hamba/avro/blob/v2.23.0/codec_fixed.go#L76), but when decoding, only big.Rat is supported (see https://github.com/hamba/avro/blob/v2.23.0/codec_fixed.go#L40).

This causes an issue when trying to encode and decode using the same struct, as it will be incompatible and not have the ability to decode into the same type that it encoded from.

The tests that cover this scenario (here https://github.com/hamba/avro/blob/v2.23.0/decoder_fixed_test.go#L43) don't correctly pass in the destination object, since Decode requires a pointer to where we want to store the data. Since we want to store in a *big.Rat (as that's what we encoded from), we need to pass in a **big.Rat.

@nrwiersma
Copy link
Member

I have seen the oddness around big.Rat but was not aware it actually manifested a bug. Thanks for catching this.

@nrwiersma
Copy link
Member

Would it be possible for you to test this branch: https://github.com/hamba/avro/tree/fix-big-rat

@cdrexle
Copy link
Author

cdrexle commented Jul 31, 2024

Thanks for the quick response! Yes that fixed the issue 👍 I was working on a PR myself that was pretty much what you have here

@nrwiersma
Copy link
Member

Out of interest, is this being testing in a production like workload or in a Go test setup?

@cdrexle
Copy link
Author

cdrexle commented Aug 1, 2024

Thanks for merging this in! Any idea when a new tag will be cut?

This is being tested in a go test setup, that writes an ocf file and immediately reads it out, using the same struct type for both read and write.

@nrwiersma
Copy link
Member

More than likely early next week some time.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants