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

Handle overflow in GCD scalar function #11053

Closed
LorrensP-2158466 opened this issue Jun 21, 2024 · 1 comment · Fixed by #11057
Closed

Handle overflow in GCD scalar function #11053

LorrensP-2158466 opened this issue Jun 21, 2024 · 1 comment · Fixed by #11057
Assignees
Labels
bug Something isn't working

Comments

@LorrensP-2158466
Copy link
Contributor

LorrensP-2158466 commented Jun 21, 2024

Describe the bug

This is a follow-up on issue #11031 and PR #11036, which fixed the incorrect results or infinite loops caused by passing i64::MIN in combination with other values into the GCD scalar function.
While fixing this, we noticed that an overflow occurred when passing i64::MIN in both parameters.
This can also happen in the LCM function, because it depends on GCD.

To Reproduce

DataFusion CLI v39.0.0

> select gcd(-9223372036854775808, -9223372036854775808);
+--------------------------------------------------------------+
| gcd(Int64(-9223372036854775808),Int64(-9223372036854775808)) |
+--------------------------------------------------------------+
| -9223372036854775808                                         |
+--------------------------------------------------------------+
1 row(s) fetched.

Expected behavior

The real GCD of i64::MIN and i64::MIN is 9223372036854775808, but this can't be represented as an i64 (it is i64::MAX+1), thus we need to return an overflow error.
For example like this:

i64::try_from(a << shift).map_err(|_| exec_datafusion_err!("Overflow in gcd"))

Additional context

Credits to @jonahgao (sorry for the ping, but it is owed)

@LorrensP-2158466 LorrensP-2158466 added the bug Something isn't working label Jun 21, 2024
@LorrensP-2158466
Copy link
Contributor Author

take

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant