-
Notifications
You must be signed in to change notification settings - Fork 95
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
int is unsupported for Avro long #421
Comments
|
Right, when decoding this can be unsafe, but encoding should be safe either way. What do you think about this approach, which ensures that decoding is only allowed on 64-bit systems? |
This seems like a good option to me. There will need an explanation in the README to explain this though. |
Ok, I opened the PR and added a note to the readme, let me know what you think. And thanks for the quick response @nrwiersma! 🙇 |
Thanks for the contribution. |
I think there's an issue with safely encoding values of the Go type
int
on 64-bit systems. The type can contain values up tomath.MaxInt64
, while only values up tomath.MaxInt32
can be safely encoded in the Avro typeint
. The type could safely get encoded intolong
, but the library doesn't support that.You can find a reproducible test case here: https://gist.github.com/lovromazgon/50ecf204e65f7d1ad080fbd0fa7d52fb
The test tries to encode and decode a Go value of type
int
containing2147483648
(i.e.math.MaxInt32 1
) with two schemas:int
it succeeds, but the decoded value doesn't match (overflow)long
it fails becauseint is unsupported for Avro long
The first case is a possible footgun, it's debatable if you want to add an option for the encoder to be "strict" and disable the support for encoding values of Go type
int
into Avro typeint
.I personally care more about the second case, encoding a Go
int
into the Avro typelong
should work.Let me know what you think, I can try to work on this and open a PR with a fix.
Possibly related to #59.
The text was updated successfully, but these errors were encountered: