Skip to content

throw error on number overflow #25

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

Closed
wants to merge 1 commit into from
Closed

throw error on number overflow #25

wants to merge 1 commit into from

Conversation

karlmcguire
Copy link

No description provided.

@@ -38,6 +39,12 @@ func parse_number_simd(buf []byte, found_minus bool) (success, is_double bool, d
}

var err error
// check if number would overflow
_, err = strconv.ParseUint(string(buf[:pos]), 10, 64)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems to prevent big numbers to be returned as floats.

These 2 unit tests fail when this is in place:

https://github.com/karlmcguire/simdjson-go/blob/e09093318f0c57c1bf817744ca1e823d1f29011a/simdjson_amd64_test.go#L933-L946

So this cannot be used as-is. I will take a look at this piece of code and see what can be done.

@klauspost
Copy link
Collaborator

You led me down quite a rabbit hole.

See #27

If you need integers for specific fields you will have to check the returned types, however now it will also return uint64s in case they fit.

@klauspost klauspost closed this Jan 18, 2021
klauspost added a commit to klauspost/simdjson-go-1 that referenced this pull request Jan 22, 2021
As flag to tape that indicates that an integer was converted to float due to int64/uint64 limits.

Fixes minio#25
harshavardhana pushed a commit that referenced this pull request Jan 25, 2021
As flag to tape that indicates that an integer was converted to float due to int64/uint64 limits.

Fixes #25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants