Fix regression for negative-scale decimal128 in log

datafusion/functions/src/math/log.rs

@@ -116,13 +116,25 @@ fn log_decimal128(value: i128, scale: i8, base: f64) -> Result<f64, ArrowError>
         )));
     }
 
-    let unscaled_value = decimal128_to_i128(value, scale)?;
-    if unscaled_value > 0 {
-        let log_value: u32 = unscaled_value.ilog(base as i128);
-        Ok(log_value as f64)
+    if scale < 0 {
+        if value > 0 {
+            // Compute actual value: value * 10^(-scale) = value * 10^|scale|
+            let actual_value = (value as f64) * 10.0_f64.powi(-scale as i32);
+            Ok(actual_value.log(base))
+        } else {
+            // Reflect f64::log behaviour
+            Ok(f64::NAN)
+        }
     } else {
-        // Reflect f64::log behaviour
-        Ok(f64::NAN)
+        // For non-negative scales, use existing logic
+        let unscaled_value = decimal128_to_i128(value, scale)?;
+        if unscaled_value > 0 {
+            let log_value: u32 = unscaled_value.ilog(base as i128);
+            Ok(log_value as f64)
+        } else {
+            // Reflect f64::log behaviour
+            Ok(f64::NAN)
+        }
     }
 }
 
@@ -291,43 +303,78 @@ impl ScalarUDFImpl for LogFunc {
         }
         let number = args.pop().unwrap();
         let number_datatype = arg_types.pop().unwrap();
-        // default to base 10
-        let base = if let Some(base) = args.pop() {
-            base
-        } else {
-            lit(ScalarValue::new_ten(&number_datatype)?)
+
+        let has_negative_scale = match &number_datatype {
+            DataType::Decimal32(_, scale)
+            | DataType::Decimal64(_, scale)
+            | DataType::Decimal128(_, scale)
+            | DataType::Decimal256(_, scale) => *scale < 0,
+            _ => false,
         };
 
-        match number {
+        // Get base if provided, otherwise None (will create default base 10 if needed)
+        let base_option = args.pop();
+
+        // Helper to construct result args (used in multiple places)
+        let make_result_args = |base: Expr| -> Result<Vec<Expr>> {
+            Ok(match num_args {
+                1 => vec![number.clone()],
+                2 => vec![base, number.clone()],
+                _ => {
+                    return internal_err!(
+                        "Unexpected number of arguments in log::simplify"
+                    );
+                }
+            })
+        };
+
+        match &number {
             Expr::Literal(value, _)
-                if value == ScalarValue::new_one(&number_datatype)? =>
+                if !has_negative_scale
+                    && *value == ScalarValue::new_one(&number_datatype)? =>
             {
+                let base = base_option.unwrap_or_else(|| {
+                    lit(ScalarValue::new_ten(&number_datatype).unwrap())
+                });
                 Ok(ExprSimplifyResult::Simplified(lit(ScalarValue::new_zero(
                     &info.get_data_type(&base)?,
                 )?)))
             }
-            Expr::ScalarFunction(ScalarFunction { func, mut args })
-                if is_pow(&func) && args.len() == 2 && base == args[0] =>
-            {
-                let b = args.pop().unwrap(); // length checked above
-                Ok(ExprSimplifyResult::Simplified(b))
+            Expr::ScalarFunction(ScalarFunction {
+                func,
+                args: pow_args,
+            }) if !has_negative_scale && is_pow(func) && pow_args.len() == 2 => {
+                let base = base_option.unwrap_or_else(|| {
+                    lit(ScalarValue::new_ten(&number_datatype).unwrap())
+                });
+                if base == pow_args[0] {
+                    Ok(ExprSimplifyResult::Simplified(pow_args[1].clone()))
+                } else {
+                    Ok(ExprSimplifyResult::Original(make_result_args(base)?))
+                }
             }
-            number => {
+            _ => {
+                // Handle negative scale or when simplification doesn't apply
+                if has_negative_scale {
+                    // For 1-arg case, base_option is None, so return early
+                    if base_option.is_none() {
+                        return Ok(ExprSimplifyResult::Original(vec![number.clone()]));
+                    }
+                    // For 2-arg case, use the provided base
+                    let base = base_option.unwrap();
+                    return Ok(ExprSimplifyResult::Original(make_result_args(base)?));
+                }
+
+                let base = base_option.unwrap_or_else(|| {
+                    lit(ScalarValue::new_ten(&number_datatype).unwrap())
+                });
+
                 if number == base {
                     Ok(ExprSimplifyResult::Simplified(lit(ScalarValue::new_one(
                         &number_datatype,
                     )?)))
                 } else {
-                    let args = match num_args {
-                        1 => vec![number],
-                        2 => vec![base, number],
-                        _ => {
-                            return internal_err!(
-                                "Unexpected number of arguments in log::simplify"
-                            );
-                        }
-                    };
-                    Ok(ExprSimplifyResult::Original(args))
+                    Ok(ExprSimplifyResult::Original(make_result_args(base)?))
                 }
             }
         }

datafusion/sqllogictest/test_files/decimal.slt

@@ -918,6 +918,58 @@ select log(2.0, null);
 ----
 NULL
 
+# log with negative scale decimals
+# Using scientific notation to create decimals with negative scales
+# 1e4 = 10000 with scale -4, log10(10000) = 4.0
+query R
+select log(1e4);
+----
+4
+
+# log with negative scale and explicit base 10
+query R
+select log(10, 1e4);
+----
+4
+
+# log with negative scale and base 2
+# 8e1 = 80 with scale -1, log2(80) ≈ 6.321928
+query R
+select log(2.0, 8e1);
+----
+6.321928094887
+
+# log with negative scale and base 2 (another value)
+# 16e1 = 160 with scale -1, log2(160) ≈ 7.321928
+query R
+select log(2.0, 16e1);
+----
+7.321928094887
+
+# log with negative scale -3
+# 5e3 = 5000 with scale -3, log10(5000) ≈ 3.69897
+query R
+select log(5e3);
+----
+3.698970004336
+
+# log with negative scale array values
+query R rowsort
+select log(value) from (values (1e3), (1e4), (1e5)) as t(value);
+----
+3
+4
+5
+
+# log with negative scale and different bases
+# Note: Results may come in different order depending on execution
+query R
+select log(base, 1e4) from (values (10.0), (2.0), (3.0)) as t(base) order by base ;
+----
+13.287712379549
+8.383613097158
+4
+
 # power with decimals
 
 query RT