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

SQL: Fix MOD() for long and integer arguments #36599

Merged
merged 1 commit into from Dec 13, 2018

Conversation

matriv
Copy link
Contributor

@matriv matriv commented Dec 13, 2018

Previously, Math.floorMod was used for integers and longs
which has different logic for negative numbers. Also, the
priority of data types check was wrong as if one of the args
is double the evaluation should be with doubles, then for floats,
then longs and finally integers.

Fixes: #36364

Previously, Math.floorMod was used for integers and longs
which has different logic for negative numbers. Also, the
priority of data types check was wrong as if one of the args
is double the evaluation should be with doubles, then for floats,
then longs and finally integers.

Fixes: elastic#36364
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

Copy link
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@costin costin left a comment

Choose a reason for hiding this comment

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

LGTM.

P.S. What's up with removing the throws Exception from the methods?

@matriv
Copy link
Contributor Author

matriv commented Dec 13, 2018

@costin you get a warning for them in the IDE, since there is no signature requiring this and doesn't change anything in the behaviour of junit.

@matriv matriv merged commit 730b154 into elastic:master Dec 13, 2018
@matriv matriv deleted the mt/fix-36364 branch December 13, 2018 22:59
matriv added a commit that referenced this pull request Dec 13, 2018
Previously, Math.floorMod was used for integers and longs
which has different logic for negative numbers. Also, the
priority of data types check was wrong as if one of the args
is double the evaluation should be with doubles, then for floats,
then longs and finally integers.

Fixes: #36364
@matriv
Copy link
Contributor Author

matriv commented Dec 13, 2018

Backported to 6.x with dbf13e0

matriv added a commit that referenced this pull request Dec 13, 2018
Previously, Math.floorMod was used for integers and longs
which has different logic for negative numbers. Also, the
priority of data types check was wrong as if one of the args
is double the evaluation should be with doubles, then for floats,
then longs and finally integers.

Fixes: #36364
@matriv
Copy link
Contributor Author

matriv commented Dec 13, 2018

Backported to 6.5 with bf95f55

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

Successfully merging this pull request may close these issues.

None yet

5 participants