Copilot commented on code in PR #19303:
URL: https://github.com/apache/datafusion/pull/19303#discussion_r2615228891
##########
datafusion/functions/src/math/power.rs:
##########
@@ -647,4 +671,239 @@ mod tests {
"Not yet implemented: Negative scale is not yet supported value:
-1"
);
}
+
+ #[test]
+ fn test_power_coerce_types() {
+ let power_func = PowerFunc::new();
+
+ // Int64 base with Int64 exponent -> both stay Int64
+ let result = power_func
+ .coerce_types(&[DataType::Int64, DataType::Int64])
+ .unwrap();
+ assert_eq!(result, vec![DataType::Int64, DataType::Int64]);
+
+ // Float64 base with Float64 exponent -> both stay Float64
+ let result = power_func
+ .coerce_types(&[DataType::Float64, DataType::Float64])
+ .unwrap();
+ assert_eq!(result, vec![DataType::Float64, DataType::Float64]);
+
+ // Int64 base with Float64 exponent -> base coerced to Float64
+ // This is needed because integer power doesn't support
negative/fractional exponents
+ let result = power_func
+ .coerce_types(&[DataType::Int64, DataType::Float64])
+ .unwrap();
+ assert_eq!(result, vec![DataType::Float64, DataType::Float64]);
+
+ // Int32 base with Float32 exponent -> both coerced to Float64
+ let result = power_func
+ .coerce_types(&[DataType::Int32, DataType::Float32])
+ .unwrap();
+ assert_eq!(result, vec![DataType::Float64, DataType::Float64]);
+
+ // Null base with Float64 exponent -> base coerced to Float64
+ let result = power_func
+ .coerce_types(&[DataType::Null, DataType::Float64])
+ .unwrap();
+ assert_eq!(result, vec![DataType::Float64, DataType::Float64]);
+
+ // Null base with Int64 exponent -> base stays Int64
+ let result = power_func
+ .coerce_types(&[DataType::Null, DataType::Int64])
+ .unwrap();
+ assert_eq!(result, vec![DataType::Int64, DataType::Int64]);
+ }
+
+ /// 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() {
+ // 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")
+ }
+ }
+ }
Review Comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
