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

Force all values to float64 for math #3611

Closed
wants to merge 3 commits into from
Closed

Force all values to float64 for math #3611

wants to merge 3 commits into from

Conversation

otoolep
Copy link
Contributor

@otoolep otoolep commented Aug 10, 2015

Fixes issue #3000

Any literal values in a math query e.g. the '2' in 'value * 2' are stored as float64 by the parser, so all other values must be cast to this type. This does mean that precision may be lost if the integer values are greater than 2^53.

@otoolep otoolep force-pushed the integer_math branch 3 times, most recently from 2e98868 to 5e3cbec Compare August 10, 2015 19:57
@otoolep otoolep changed the title Add system-level test for math Force all values to float64 for math Aug 10, 2015
Any literal values in a math query e.g. the '2' in 'value * 2' are
stored as float64, so all other values must be cast to this type. This
does mean that precision may be lost if the integer values are greater
than 2^53.
@pauldix
Copy link
Member

pauldix commented Aug 10, 2015

Wouldn't it be better to fix it so that if the field is an int we use int instead of float?

@otoolep
Copy link
Contributor Author

otoolep commented Aug 10, 2015

@pauldix - as explained in the commit, the parser stores the literal numbers in the math as float64. So the query value * 2 always has 2 has a float64. We're stuck with float math regardless of the value type. We could fix this, but it would be a bigger change -- possibly to the parser. I'm not sure we want to do this this so close to the 0.9.3 release.

Does this answer your question?

@otoolep
Copy link
Contributor Author

otoolep commented Aug 10, 2015

There is something up with Circle and the git history -- it can't check out the code. I have created an identical PR here:

#3613

@otoolep otoolep closed this Aug 10, 2015
@pauldix
Copy link
Member

pauldix commented Aug 10, 2015

Yeah, I'm suggesting that we actually fix it so that it will do the right math on the given type. Guess this will work for a quick fix, but we should open up an issue to fix this later.

@otoolep
Copy link
Contributor Author

otoolep commented Aug 10, 2015

#3614 opened to track parsing the mathematical literals correctly.

@otoolep otoolep deleted the integer_math branch August 10, 2015 20: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.

None yet

2 participants