Skip to content

Fix overflow in pow #11124

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jun 26, 2024
Merged

Fix overflow in pow #11124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d785779
checked overflow on integer parameter
LorrensP-2158466 Jun 25, 2024
36ba216
added slt test
LorrensP-2158466 Jun 25, 2024
6a32515
fix: forgot to remove failling test case
LorrensP-2158466 Jun 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 23 additions & 10 deletions datafusion/functions/src/math/power.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@

//! Math function: `power()`.

use arrow::datatypes::DataType;
use arrow::datatypes::{ArrowNativeTypeOp, DataType};

use datafusion_common::{
exec_err, plan_datafusion_err, DataFusionError, Result, ScalarValue,
arrow_datafusion_err, exec_datafusion_err, exec_err, plan_datafusion_err,
DataFusionError, Result, ScalarValue,
};
use datafusion_expr::expr::ScalarFunction;
use datafusion_expr::simplify::{ExprSimplifyResult, SimplifyInfo};
Expand Down Expand Up @@ -94,14 +96,25 @@ impl ScalarUDFImpl for PowerFunc {
{ f64::powf }
)),

DataType::Int64 => Arc::new(make_function_inputs2!(
&args[0],
&args[1],
"base",
"exponent",
Int64Array,
{ i64::pow }
)),
DataType::Int64 => {
let bases = downcast_arg!(&args[0], "base", Int64Array);
let exponents = downcast_arg!(&args[1], "exponent", Int64Array);
bases
.iter()
.zip(exponents.iter())
.map(|(base, exp)| match (base, exp) {
(Some(base), Some(exp)) => Ok(Some(base.pow_checked(
exp.try_into().map_err(|_| {
exec_datafusion_err!(
"Can't use negative exponents: {exp} in integer computation, please use Float."
Copy link
Contributor

@alamb alamb Jun 26, 2024

Choose a reason for hiding this comment

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

💯 for the error message

)
})?,
).map_err(|e| arrow_datafusion_err!(e))?)),
_ => Ok(None),
})
.collect::<Result<Int64Array>>()
.map(Arc::new)? as ArrayRef
}

other => {
return exec_err!(
Expand Down
3 changes: 3 additions & 0 deletions datafusion/sqllogictest/test_files/math.slt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -575,3 +575,6 @@ select gcd(-9223372036854775808, -9223372036854775808);

query error DataFusion error: Arrow error: Compute error: Signed integer overflow in LCM\(\-9223372036854775808, \-9223372036854775808\)
select lcm(-9223372036854775808, -9223372036854775808);

query error DataFusion error: Arrow error: Compute error: Overflow happened on: 2107754225 \^ 1221660777
select power(2107754225, 1221660777);