Yuvraj-cyborg commented on code in PR #19369:
URL: https://github.com/apache/datafusion/pull/19369#discussion_r2627228252
##########
datafusion/functions/src/math/power.rs:
##########
@@ -149,22 +151,112 @@ where
/// Binary function to calculate a math power to float exponent
/// for scaled integer types.
-/// Returns error if exponent is negative or non-integer, or base invalid
fn pow_decimal_float<T>(base: T, scale: i8, exp: f64) -> Result<T, ArrowError>
where
T: From<i32> + ArrowNativeTypeOp,
{
- if !exp.is_finite() || exp.trunc() != exp {
+ if exp.is_finite() && exp.trunc() == exp && exp >= 0f64 && exp < u32::MAX
as f64 {
+ return pow_decimal_int(base, scale, exp as i64);
+ }
+
+ if !exp.is_finite() {
return Err(ArrowError::ComputeError(format!(
- "Cannot use non-integer exp: {exp}"
+ "Cannot use non-finite exp: {exp}"
+ )));
+ }
+
+ pow_decimal_float_fallback(base, scale, exp)
+}
+
+/// Fallback implementation using f64 for negative or non-integer exponents.
+/// This handles cases that cannot be computed using integer arithmetic.
+fn pow_decimal_float_fallback<T>(base: T, scale: i8, exp: f64) -> Result<T,
ArrowError>
+where
+ T: From<i32> + ArrowNativeTypeOp,
+{
+ let scale_factor = 10f64.powi(scale as i32);
+ let base_f64 = format!("{base:?}")
+ .parse::<f64>()
+ .map(|v| v / scale_factor)
+ .map_err(|_| {
+ ArrowError::ComputeError(format!("Cannot convert base {base:?} to
f64"))
+ })?;
+
+ let result_f64 = base_f64.powf(exp);
+
+ if !result_f64.is_finite() {
+ return Err(ArrowError::ArithmeticOverflow(format!(
+ "Result of {base_f64}^{exp} is not finite"
)));
}
- if exp < 0f64 || exp >= u32::MAX as f64 {
+
+ let result_scaled = result_f64 * scale_factor;
+ let result_rounded = result_scaled.round();
+
+ if result_rounded.abs() > i128::MAX as f64 {
return Err(ArrowError::ArithmeticOverflow(format!(
- "Unsupported exp value: {exp}"
+ "Result {result_rounded} is too large for the target decimal type"
)));
}
- pow_decimal_int(base, scale, exp as i64)
+
+ decimal_from_i128::<T>(result_rounded as i128)
+}
+
+fn decimal_from_i128<T>(value: i128) -> Result<T, ArrowError>
Review Comment:
practically the f64 computation already loses precision past ~10^15
significant digits , isn't it ??
That's why I thought it isn't needed..
If you say then I can add that too
like add a Decimal256 path.
--
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]
