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.

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

Unmarshal JSON numbers as ints by default #2091

Merged
merged 1 commit into from
Jul 26, 2016

Conversation

tsg
Copy link
Contributor

@tsg tsg commented Jul 25, 2016

The change modifies the unmarshaling logic to try parsing the numbers
in an int64 first, and only on error parse to a float64. In practice,
this means that e.g. 1 will become an int64 and 1.0 will become a
float64.

Fixes #2038

@tsg tsg added in progress Pull request is currently in progress. review labels Jul 25, 2016
@tsg tsg force-pushed the json_unmarshal_ints branch 2 times, most recently from 1eef37a to a94e15a Compare July 25, 2016 20:32
The change modifies the unmarshaling logic to try parsing the numbers
in an int64 first, and only on error parse to a float64. In practice,
this means that e.g. 1 will become an int64 and 1.0 will become a
float64.

Fixes elastic#2038. Includes a system test to verify that ticket.
@tsg tsg force-pushed the json_unmarshal_ints branch from a94e15a to 382997f Compare July 25, 2016 20:58
@tsg tsg added docs Filebeat Filebeat and removed in progress Pull request is currently in progress. docs labels Jul 25, 2016
@andrewkroh
Copy link
Member

LGTM

@andrewkroh andrewkroh merged commit 621a2f3 into elastic:master Jul 26, 2016
tsg pushed a commit to tsg/beats that referenced this pull request Jul 26, 2016
I realized that when rebasing elastic#2091 on top of elastic#2088 a JSON test was
lost. This resurrects it.
ruflin pushed a commit that referenced this pull request Jul 26, 2016
I realized that when rebasing #2091 on top of #2088 a JSON test was
lost. This resurrects it.
@tsg tsg deleted the json_unmarshal_ints branch August 25, 2016 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants