fix: pow() with integer base and negative float exponent returns error #19303

adriangb · 2025-12-12T18:38:26Z

Which issue does this PR close?

Closes #19314

Fixes a regression introduced in #18032 where pow(10, -2.0) fails with:

Arrow error: Arithmetic overflow: Exponent -2 in integer computation is out of bounds.

Rationale for this change

After #18032 added decimal support to the power() function, the custom type coercion kept integer bases as Int64 while float exponents became Float64. However, invoke_with_args() assumed Int64 exponent for Int64 base and used integer power computation, which doesn't support negative exponents.

What changes are included in this PR?

Modified coerce_types() in datafusion/functions/src/math/power.rs to coerce integer bases to Float64 when the exponent is Float64, since floating-point arithmetic is required for negative/fractional exponents.

This matches the original behavior before #18032 where mixed types would both be coerced to Float64.

Are these changes tested?

Yes, added comprehensive tests:

test_power_coerce_types - verifies type coercion behavior
test_power_int_base_negative_float_exp - tests the specific regression case
test_power_edge_cases_f64 - tests edge cases for float power (zero exponent, one base, negative base, NaN)
test_power_edge_cases_i64 - tests edge cases for integer power
test_power_i64_negative_exp_fails - verifies integer power with negative exponent fails
test_power_zero_base_negative_exp - tests infinity result

Are there any user-facing changes?

No breaking changes. This restores the expected behavior of pow() with integer base and float exponent.

🤖 Generated with Claude Code

adriangb · 2025-12-12T18:40:49Z

@theirix could you take a look at this since I think it was your change that this is patching?

adriangb · 2025-12-12T18:42:26Z

Btw I gave Claude the bug and had it do a bisect then fix, I only checked that tests pass and that there is a regression test. The code does look reasonable to me, but I'm not too familiar with this part of the codebase.

Copilot

Pull request overview

This PR fixes a regression introduced in #18032 where pow(10, -2.0) fails with an arithmetic overflow error when an integer base is used with a negative float exponent. The fix modifies the type coercion logic to convert integer bases to Float64 when the exponent is Float64, ensuring that floating-point power computation is used instead of integer power computation which doesn't support negative or fractional exponents.

Key changes:

Refactored coerce_types() to determine exponent type first, then use it to inform base type coercion
Integer bases are now coerced to Float64 when paired with Float64 exponents
Added comprehensive test coverage for type coercion, edge cases, and the specific regression scenario

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-12T18:45:27Z

datafusion/functions/src/math/power.rs

+    fn test_power_int_base_negative_float_exp() {
+        // After coercion, Int64 base with Float64 exponent becomes Float64, Float64
+        let arg_fields = vec![
+            Field::new("a", DataType::Float64, true).into(),
+            Field::new("b", DataType::Float64, true).into(),
+        ];
+        let args = ScalarFunctionArgs {
+            args: vec![
+                ColumnarValue::Array(Arc::new(Float64Array::from(vec![10.0, 2.0, 4.0]))),
+                ColumnarValue::Array(Arc::new(Float64Array::from(vec![-2.0, -1.0, 0.5]))),
+            ],
+            arg_fields,
+            number_rows: 3,
+            return_field: Field::new("f", DataType::Float64, true).into(),
+            config_options: Arc::new(ConfigOptions::default()),
+        };
+        let result = PowerFunc::new()
+            .invoke_with_args(args)
+            .expect("failed to initialize function power");
+
+        match result {
+            ColumnarValue::Array(arr) => {
+                let floats = as_float64_array(&arr)
+                    .expect("failed to convert result to a Float64Array");
+                assert_eq!(floats.len(), 3);
+                assert!((floats.value(0) - 0.01).abs() < 1e-10); // 10^-2 = 0.01
+                assert!((floats.value(1) - 0.5).abs() < 1e-10); // 2^-1 = 0.5
+                assert!((floats.value(2) - 2.0).abs() < 1e-10); // 4^0.5 = 2.0
+            }
+            ColumnarValue::Scalar(_) => {
+                panic!("Expected an array value")
+            }
+        }
+    }


This test doesn't actually verify the type coercion fix that this PR introduces. The test uses Float64 arrays directly, but to properly test the regression fix, it should use Int64 for the base and Float64 for the exponent at the SQL/expression level (before coercion). The current test only verifies that Float64 power computation works correctly, not that Int64 bases are properly coerced to Float64 when paired with Float64 exponents. Consider adding an integration test or modifying this test to verify the coercion behavior end-to-end.

Good point. I'll add an SLT test.

theirix · 2025-12-12T18:47:55Z

@theirix could you take a look at this since I think it was your change that this is patching?

Yes, I recall this behaviour as we discussed a similar problem with @Jefffrey #18032 (comment) . Let me review

theirix · 2025-12-13T09:14:26Z

datafusion/functions/src/math/power.rs

+        ) -> Result<DataType> {
            match data_type {
-                DataType::Null => Ok(DataType::Int64),
+                DataType::Null => {


Can we group these two arms closer? Different comments are valuable, so I would suggest keeping them.

theirix · 2025-12-13T09:19:12Z

datafusion/sqllogictest/test_files/math.slt

 select lcm(2, 9223372036854775803);


+## pow/power


I've placed a TODO
# TODO: check backward compatibility for a case with base in64 and exponent float64 since the power coercion is introduced
in decimal.slt:948, and they are testing datafusion.sql_parser.parse_float_as_decimal = true behaviour.

Tests in math.slt test pure float behaviour for this specific issue. Unfortunately, decimal behaviour is not triggered. Can we port a few of them to the decimal.slt to test the new flag?

If you've covered this case, let's remove that TODO.

theirix · 2025-12-13T09:24:53Z

datafusion/functions/src/math/power.rs

+    /// Test for issue where pow(10, -2.0) was failing with overflow error
+    /// when integer base was used with negative float exponent
+    #[test]
+    fn test_power_int_base_negative_float_exp() {


We can omit it if it tested by SELECT pow(2, -0.5) SLT

I think the only remaining SLT test now is pow(2, -2)

theirix · 2025-12-13T09:25:10Z

datafusion/functions/src/math/power.rs

    }
+
+    #[test]
+    fn test_power_coerce_types() {


I like this, very useful unit test

theirix · 2025-12-13T09:26:54Z

datafusion/functions/src/math/power.rs

+    /// Test edge cases: zero exponent (anything^0 = 1), one base (1^anything = 1),
+    /// zero base, and negative bases
+    #[test]
+    fn test_power_edge_cases_f64() {


It could be better to convert it to SLT, too.

We tried to introduce unit tests to test different code paths (e.g., array vs. scalar, decimal vs. int), but edge cases with data are usually tested better via an SLT.

When calling pow(10, -2.0), the power function would fail with: "Arrow error: Arithmetic overflow: Exponent -2 in integer computation is out of bounds." This was a regression introduced in apache#18032 which added decimal support to power(). The issue was that the custom type coercion kept integer bases as Int64 while float exponents became Float64, but the invoke_with_args() assumed Int64 exponent for Int64 base, which doesn't support negative exponents. The fix modifies coerce_types() to coerce integer bases to Float64 when the exponent is Float64, since floating-point arithmetic is required for negative/fractional exponents. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Jefffrey

I was hoping to get #18968 merged, so we wouldn't need to add more logic to the coerce_types function which ideally we're aiming to remove 🤔

adriangb · 2025-12-13T16:21:29Z

@theirix this now works by always casting exponents to float, this is the same behavior as Postgres:

select pg_typeof(pow(2, 3));
    pg_typeof     
------------------
 double precision
(1 row)

Here is the full table of behavior:

  | Base    | Exponent | DataFusion | PostgreSQL         |
  |---------|----------|------------|--------------------|
  | int     | int      | Float64    | double precision ✓ |
  | int     | float    | Float64    | double precision ✓ |
  | decimal | int      | Decimal128 | numeric ✓          |
  | decimal | float    | Decimal128 | numeric ✓          |

The only difference I can see is that pow(2.5, 2) returns numeric in postgres and float64 in datafusion, but this is because select pg_typeof(2.5) is numeric.

adriangb · 2025-12-13T16:30:21Z

I was hoping to get #18968 merged, so we wouldn't need to add more logic to the coerce_types function which ideally we're aiming to remove 🤔

I'm happy to defer to that I was just trying to be helpful and fix a regression. Maybe you can port some of the tests over?

theirix · 2025-12-13T18:25:46Z

@theirix this now works by always casting exponents to float, this is the same behavior as Postgres:

select pg_typeof(pow(2, 3));
    pg_typeof     
------------------
 double precision
(1 row)

Here is the full table of behavior:

  | Base    | Exponent | DataFusion | PostgreSQL         |
  |---------|----------|------------|--------------------|
  | int     | int      | Float64    | double precision ✓ |
  | int     | float    | Float64    | double precision ✓ |
  | decimal | int      | Decimal128 | numeric ✓          |
  | decimal | float    | Decimal128 | numeric ✓          |

Great, it makes sense.

The only difference I can see is that pow(2.5, 2) returns numeric in postgres and float64 in datafusion, but this is because select pg_typeof(2.5) is numeric.

Yes. I suppose that after parse_float_as_decimal Datafusion's behaviour will be aligned to Postgres automatically.

Jefffrey

I haven't had time to look closely at this yet (hopefully soon), but my main concern is do we also need to remove some code paths in invoke?

For example, will this path be reachable anymore:

datafusion/datafusion/functions/src/math/power.rs

Lines 217 to 228 in fedddbc

    
           (DataType::Int64, _) => { 
        
               calculate_binary_math::<Int64Type, Int64Type, Int64Type, _>( 
        
                   &base, 
        
                   exponent, 
        
                   |b, e| match e.try_into() { 
        
                       Ok(exp_u32) => b.pow_checked(exp_u32), 
        
                       Err(_) => Err(ArrowError::ArithmeticOverflow(format!( 
        
                           "Exponent {e} in integer computation is out of bounds." 
        
                       ))), 
        
                   }, 
        
               )? 
        
           }

Also I took the liberty of creating an issue for this PR since I think it's a significant enough bug to warrant an issue for it: #19314

alamb

Thanks @adriangb

adriangb · 2025-12-14T16:53:16Z

Thanks @alamb. I plan to wait for @Jefffrey to review.

github-actions bot added the functions Changes to functions implementation label Dec 12, 2025

adriangb requested a review from Jefffrey December 12, 2025 18:40

adriangb requested a review from Copilot December 12, 2025 18:42

Copilot started reviewing on behalf of adriangb December 12, 2025 18:42 View session

Copilot AI reviewed Dec 12, 2025

View reviewed changes

github-actions bot added the sqllogictest SQL Logic Tests (.slt) label Dec 12, 2025

theirix reviewed Dec 13, 2025

View reviewed changes

adriangb and others added 5 commits December 13, 2025 09:29

Add slt tests

6c13d5d

fix query types?

e737952

address feedback

b0ee753

always make a float

55c643f

adriangb force-pushed the fix-power-negative-float-exp branch from 12b6c51 to 55c643f Compare December 13, 2025 16:14

Jefffrey reviewed Dec 13, 2025

View reviewed changes

Jefffrey reviewed Dec 14, 2025

View reviewed changes

Jefffrey mentioned this pull request Dec 14, 2025

Refactor power() signature away from user defined #18968

Merged

alamb approved these changes Dec 14, 2025

View reviewed changes

adriangb added 2 commits December 14, 2025 10:48

update test

74d1275

remove dead code

f63d153

Jefffrey approved these changes Dec 15, 2025

View reviewed changes

adriangb added this pull request to the merge queue Dec 15, 2025

Merged via the queue into apache:main with commit dbf9265 Dec 15, 2025
27 checks passed

	(DataType::Int64, _) => {
	calculate_binary_math::<Int64Type, Int64Type, Int64Type, _>(
	&base,
	exponent,
	\|b, e\| match e.try_into() {
	Ok(exp_u32) => b.pow_checked(exp_u32),
	Err(_) => Err(ArrowError::ArithmeticOverflow(format!(
	"Exponent {e} in integer computation is out of bounds."
	))),
	},
	)?
	}

fix: pow() with integer base and negative float exponent returns error #19303

fix: pow() with integer base and negative float exponent returns error #19303

Uh oh!

Conversation

adriangb commented Dec 12, 2025 • edited by Jefffrey Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

adriangb commented Dec 12, 2025

Uh oh!

adriangb commented Dec 12, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

adriangb Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

theirix commented Dec 12, 2025

Uh oh!

theirix Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

theirix Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

theirix Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

theirix Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

theirix Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

theirix Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

Jefffrey left a comment

Choose a reason for hiding this comment

Uh oh!

adriangb commented Dec 13, 2025

Uh oh!

adriangb commented Dec 13, 2025

Uh oh!

theirix commented Dec 13, 2025

Uh oh!

Jefffrey left a comment

Choose a reason for hiding this comment

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

adriangb commented Dec 14, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

adriangb commented Dec 12, 2025 •

edited by Jefffrey

Loading