Yuvraj-cyborg commented on code in PR #19372:
URL: https://github.com/apache/datafusion/pull/19372#discussion_r2659034704
##########
datafusion/functions/src/math/log.rs:
##########
@@ -104,91 +102,70 @@ impl LogFunc {
}
}
-/// Binary function to calculate logarithm of Decimal32 `value` using `base`
base
-/// Returns error if base is invalid
-fn log_decimal32(value: i32, scale: i8, base: f64) -> Result<f64, ArrowError> {
- if !base.is_finite() || base.trunc() != base {
- return Err(ArrowError::ComputeError(format!(
- "Log cannot use non-integer base: {base}"
- )));
- }
- if (base as u32) < 2 {
- return Err(ArrowError::ComputeError(format!(
- "Log base must be greater than 1: {base}"
- )));
- }
-
- let unscaled_value = decimal32_to_i32(value, scale)?;
- if unscaled_value > 0 {
- let log_value: u32 = unscaled_value.ilog(base as i32);
- Ok(log_value as f64)
- } else {
- // Reflect f64::log behaviour
- Ok(f64::NAN)
- }
+/// Checks if the base is valid for the efficient integer logarithm algorithm.
+#[inline]
+fn is_valid_integer_base(base: f64) -> bool {
+ base.trunc() == base && base >= 2.0 && base <= u32::MAX as f64
}
-/// Binary function to calculate logarithm of Decimal64 `value` using `base`
base
-/// Returns error if base is invalid
-fn log_decimal64(value: i64, scale: i8, base: f64) -> Result<f64, ArrowError> {
- if !base.is_finite() || base.trunc() != base {
- return Err(ArrowError::ComputeError(format!(
- "Log cannot use non-integer base: {base}"
- )));
- }
- if (base as u32) < 2 {
- return Err(ArrowError::ComputeError(format!(
- "Log base must be greater than 1: {base}"
- )));
+/// Generic function to calculate logarithm of a decimal value using the given
base.
+///
+/// For integer bases >= 2 with non-negative scale, uses the efficient integer
`ilog` algorithm.
+/// For all other cases (non-integer bases, negative bases, non-finite bases),
+/// falls back to f64 computation which naturally returns NaN for invalid
inputs,
+/// matching the behavior of `f64::log`.
+fn log_decimal<T>(value: T, scale: i8, base: f64) -> Result<f64, ArrowError>
+where
+ T: ToPrimitive + Copy,
+{
+ // For integer bases >= 2 and non-negative scale, try the efficient
integer algorithm
+ if is_valid_integer_base(base)
+ && scale >= 0
+ && let Some(unscaled) = unscale_decimal_value(&value, scale)
+ {
+ return if unscaled > 0 {
+ Ok(unscaled.ilog(base as u128) as f64)
+ } else {
+ Ok(f64::NAN)
+ };
}
- let unscaled_value = decimal64_to_i64(value, scale)?;
- if unscaled_value > 0 {
- let log_value: u32 = unscaled_value.ilog(base as i64);
- Ok(log_value as f64)
- } else {
- // Reflect f64::log behaviour
- Ok(f64::NAN)
- }
+ // Fallback to f64 computation for non-integer bases, negative scale, etc.
+ // This naturally returns NaN for invalid inputs (base <= 1, non-finite,
value <= 0)
+ decimal_to_f64(&value, scale).map(|v| v.log(base))
}
-/// Binary function to calculate an integer logarithm of Decimal128 `value`
using `base` base
-/// Returns error if base is invalid
-fn log_decimal128(value: i128, scale: i8, base: f64) -> Result<f64,
ArrowError> {
- if !base.is_finite() || base.trunc() != base {
- return Err(ArrowError::ComputeError(format!(
- "Log cannot use non-integer base: {base}"
- )));
- }
- if (base as u32) < 2 {
- return Err(ArrowError::ComputeError(format!(
- "Log base must be greater than 1: {base}"
- )));
- }
-
- if value <= 0 {
- // Reflect f64::log behaviour
- return Ok(f64::NAN);
- }
+/// Unscale a decimal value by dividing by 10^scale, returning the result as
u128.
+/// Returns None if the value is negative or the conversion fails.
+#[inline]
+fn unscale_decimal_value<T: ToPrimitive>(value: &T, scale: i8) -> Option<u128>
{
+ let value_u128 = value.to_u128()?;
Review Comment:
Valid ! It's overkill
--
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]
