Skip to content

[SPARK-23893][Core][SQL] Avoid possible integer overflow in multiplication #21002

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

Conversation

kiszk
Copy link
Member

@kiszk kiszk commented Apr 7, 2018

What changes were proposed in this pull request?

This PR avoids possible overflow at an operation long = (long)(int * int). The multiplication of large positive integer values may set one to MSB. This leads to a negative value in long while we expected a positive value (e.g. 0111_0000_0000_0000 * 0000_0000_0000_0010).

This PR performs long cast before the multiplication to avoid this situation.

How was this patch tested?

Existing UTs

@kiszk kiszk changed the title initial commit [SPARK-23893][Core][SQL] Avoid possible integer overflow in multiplication Apr 7, 2018
@kiszk
Copy link
Member Author

kiszk commented Apr 7, 2018

cc @gatorsmile @hvanhovell

@SparkQA
Copy link

SparkQA commented Apr 7, 2018

Test build #89012 has finished for PR 21002 at commit ac24549.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@kiszk
Copy link
Member Author

kiszk commented Apr 8, 2018

retest this please

@SparkQA
Copy link

SparkQA commented Apr 8, 2018

Test build #89020 has finished for PR 21002 at commit ac24549.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@kiszk
Copy link
Member Author

kiszk commented Apr 8, 2018

retest this please

@SparkQA
Copy link

SparkQA commented Apr 8, 2018

Test build #89024 has finished for PR 21002 at commit ac24549.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@kiszk
Copy link
Member Author

kiszk commented Apr 8, 2018

retest this please

@SparkQA
Copy link

SparkQA commented Apr 8, 2018

Test build #89030 has finished for PR 21002 at commit ac24549.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@hvanhovell
Copy link
Contributor

hvanhovell commented Apr 8, 2018

LGTM - merging to master.

@asfgit asfgit closed this in 8d40a79 Apr 8, 2018
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.

3 participants