fix: preserve precision in log() with mixed-width float arguments

datafusion/core/tests/expr_api/simplification.rs

@@ -607,14 +607,17 @@ fn test_simplify_with_cycle_count(
 #[test]
 fn test_simplify_log() {
     // Log(c3, 1) ===> 0
+    // The zero carries log's Float64 return type, not the base's Int64, so the
+    // simplified literal stays consistent with the plan schema (issue #22581).
     {
         let expr = log(col("c3_non_null"), lit(1));
-        test_simplify(expr, lit(0i64));
+        test_simplify(expr, lit(0.0f64));
     }
     // Log(c3, c3) ===> 1
+    // The one carries log's Float64 return type, not the base's Int64 (#22581).
     {
         let expr = log(col("c3_non_null"), col("c3_non_null"));
-        let expected = lit(1i64);
+        let expected = lit(1.0f64);
         test_simplify(expr, expected);
     }
     // Log(c3, Power(c3, c4)) ===> c4

datafusion/functions/src/math/log.rs

@@ -21,6 +21,7 @@ use super::power::PowerFunc;
 
 use crate::utils::calculate_binary_math;
 use arrow::array::{Array, ArrayRef};
+use arrow::compute::cast;
 use arrow::datatypes::{
     DataType, Decimal32Type, Decimal64Type, Decimal128Type, Decimal256Type, Float16Type,
     Float32Type, Float64Type,
@@ -100,6 +101,30 @@ impl LogFunc {
     }
 }
 
+/// The float type `log` computes and returns for the given argument types: the
+/// widest float present, so a wider base is never narrowed to a narrower value
+/// (issue #22581). Integer and decimal arguments are coerced to `Float64` by the
+/// signature, and null arguments do not constrain the width. When no concrete
+/// float is present the result is `Float64`.
+fn log_float_type(arg_types: &[DataType]) -> DataType {
+    let mut rank = 0u8;
+    for arg_type in arg_types {
+        let arg_rank = match arg_type {
+            DataType::Float16 => 1,
+            DataType::Float32 => 2,
+            DataType::Null => continue,
+            // Float64 and any coerced integer/decimal argument.
+            _ => 3,
+        };
+        rank = rank.max(arg_rank);
+    }
+    match rank {
+        1 => DataType::Float16,
+        2 => DataType::Float32,
+        _ => DataType::Float64,
+    }
+}
+
 /// Checks if the base is valid for the efficient integer logarithm algorithm.
 #[inline]
 fn is_valid_integer_base(base: f64) -> bool {
@@ -195,12 +220,12 @@ impl ScalarUDFImpl for LogFunc {
     }
 
     fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
-        // Check last argument (value)
-        match &arg_types.last().ok_or(plan_datafusion_err!("No args"))? {
-            DataType::Float16 => Ok(DataType::Float16),
-            DataType::Float32 => Ok(DataType::Float32),
-            _ => Ok(DataType::Float64),
+        if arg_types.is_empty() {
+            return Err(plan_datafusion_err!("No args"));
         }
+        // Compute in the widest float of all arguments so a wider base is not
+        // narrowed to the value's type (issue #22581).
+        Ok(log_float_type(arg_types))
     }
 
     fn output_ordering(&self, input: &[ExprProperties]) -> Result<SortProperties> {
@@ -247,26 +272,39 @@ impl ScalarUDFImpl for LogFunc {
         let value = value.to_array(args.number_rows)?;
 
         let output: ArrayRef = match value.data_type() {
-            DataType::Float16 => {
-                calculate_binary_math::<Float16Type, Float16Type, Float16Type, _>(
-                    &value,
-                    &base,
-                    |value, base| Ok(value.log(base)),
-                )?
-            }
-            DataType::Float32 => {
-                calculate_binary_math::<Float32Type, Float32Type, Float32Type, _>(
-                    &value,
-                    &base,
-                    |value, base| Ok(value.log(base)),
-                )?
-            }
-            DataType::Float64 => {
-                calculate_binary_math::<Float64Type, Float64Type, Float64Type, _>(
-                    &value,
-                    &base,
-                    |value, base| Ok(value.log(base)),
-                )?
+            DataType::Float16 | DataType::Float32 | DataType::Float64 => {
+                // Compute in the widest float of base and value so a wider base
+                // is not narrowed to the value's type (issue #22581).
+                let target =
+                    log_float_type(&[base.data_type(), value.data_type().clone()]);
+                let value = if value.data_type() == &target {
+                    value
+                } else {
+                    cast(&value, &target)?
+                };
+                match target {
+                    DataType::Float16 => {
+                        calculate_binary_math::<Float16Type, Float16Type, Float16Type, _>(
+                            &value,
+                            &base,
+                            |value, base| Ok(value.log(base)),
+                        )?
+                    }
+                    DataType::Float32 => {
+                        calculate_binary_math::<Float32Type, Float32Type, Float32Type, _>(
+                            &value,
+                            &base,
+                            |value, base| Ok(value.log(base)),
+                        )?
+                    }
+                    _ => {
+                        calculate_binary_math::<Float64Type, Float64Type, Float64Type, _>(
+                            &value,
+                            &base,
+                            |value, base| Ok(value.log(base)),
+                        )?
+                    }
+                }
             }
             DataType::Decimal32(_, scale) => {
                 calculate_binary_math::<Decimal32Type, Float64Type, Float64Type, _>(
@@ -363,8 +401,11 @@ impl ScalarUDFImpl for LogFunc {
             Expr::Literal(value, _)
                 if value == ScalarValue::new_one(&number_datatype)? =>
             {
+                // The zero must carry the resolved return type, not the base's,
+                // so a wider value does not leave the plan schema inconsistent
+                // (issue #22581).
                 Ok(ExprSimplifyResult::Simplified(lit(ScalarValue::new_zero(
-                    &info.get_data_type(&base)?,
+                    &return_type,
                 )?)))
             }
             Expr::ScalarFunction(ScalarFunction { func, mut args })
@@ -376,7 +417,7 @@ impl ScalarUDFImpl for LogFunc {
             number => {
                 if number == base {
                     Ok(ExprSimplifyResult::Simplified(lit(ScalarValue::new_one(
-                        &number_datatype,
+                        &return_type,
                     )?)))
                 } else {
                     let args = match num_args {
@@ -1205,4 +1246,72 @@ mod tests {
             }
         }
     }
+
+    #[test]
+    fn test_log_return_type_uses_widest_float() {
+        use DataType::*;
+        let f = LogFunc::new();
+
+        // A wider base must not be narrowed to the value's type (#22581).
+        assert_eq!(f.return_type(&[Float64, Float32]).unwrap(), Float64);
+        assert_eq!(f.return_type(&[Float32, Float64]).unwrap(), Float64);
+        assert_eq!(f.return_type(&[Float32, Float16]).unwrap(), Float32);
+        assert_eq!(f.return_type(&[Float16, Float32]).unwrap(), Float32);
+        // Homogeneous inputs keep their type.
+        assert_eq!(f.return_type(&[Float16, Float16]).unwrap(), Float16);
+        assert_eq!(f.return_type(&[Float32, Float32]).unwrap(), Float32);
+        assert_eq!(f.return_type(&[Float64, Float64]).unwrap(), Float64);
+        assert_eq!(f.return_type(&[Float32]).unwrap(), Float32);
+        // Null does not constrain the width; all-null falls back to Float64.
+        assert_eq!(f.return_type(&[Float32, Null]).unwrap(), Float32);
+        assert_eq!(f.return_type(&[Null, Float32]).unwrap(), Float32);
+        assert_eq!(f.return_type(&[Null, Null]).unwrap(), Float64);
+    }
+
+    #[test]
+    fn test_log_mixed_width_preserves_base_precision() {
+        // Issue #22581: log(Float64 base, Float32 value) narrowed the base to
+        // Float32, losing precision. 16777217 is not representable in f32.
+        let arg_fields = vec![
+            Field::new("a", DataType::Float64, false).into(),
+            Field::new("a", DataType::Float32, false).into(),
+        ];
+        let args = ScalarFunctionArgs {
+            args: vec![
+                ColumnarValue::Scalar(ScalarValue::Float64(Some(16777217.0))), // base
+                ColumnarValue::Scalar(ScalarValue::Float32(Some(2.0))),        // value
+            ],
+            arg_fields,
+            number_rows: 1,
+            return_field: Field::new("f", DataType::Float64, true).into(),
+            config_options: Arc::new(ConfigOptions::default()),
+        };
+        let result = LogFunc::new()
+            .invoke_with_args(args)
+            .expect("failed to initialize function log");
+
+        match result {
+            ColumnarValue::Array(arr) => {
+                let floats = as_float64_array(&arr)
+                    .expect("result should be a Float64Array, not narrowed to Float32");
+                assert_eq!(floats.len(), 1);
+
+                let expected = 2.0_f64.log(16777217.0);
+                assert!(
+                    (floats.value(0) - expected).abs() < 1e-15,
+                    "got {}, expected f64-precision {expected}",
+                    floats.value(0)
+                );
+                // The lossy f32 computation differs in the 8th decimal; the
+                // result must not match it.
+                let lossy = 2.0_f32.log(16777217.0_f32) as f64;
+                assert!(
+                    (floats.value(0) - lossy).abs() > 1e-9,
+                    "result {} still matches the lossy f32 value {lossy}",
+                    floats.value(0)
+                );
+            }
+            ColumnarValue::Scalar(_) => panic!("Expected an array value"),
+        }
+    }
 }

datafusion/sqllogictest/test_files/math.slt

@@ -920,22 +920,60 @@ logical_plan
 02)--TableScan: aggregate_simple projection=[]
 physical_plan DataSourceExec: file_groups={1 group: [[WORKSPACE_ROOT/datafusion/core/tests/data/aggregate_simple.csv]]}, projection=[NULL as log(NULL,aggregate_simple.c2)], file_type=csv, has_header=true
 
-# Float 16/32/64 for log
+# log result type is the widest float of base and value, so a wider base is
+# not narrowed to the value's type (#22581). Here base 2.5 is Float64.
 query RT
 SELECT log(2.5, arrow_cast(10.9, 'Float16')), arrow_typeof(log(2.5, arrow_cast(10.9, 'Float16')));
 ----
-2.6074219 Float16
+2.606835742462 Float64
 
 query RT
 SELECT log(2.5, 10.9::float), arrow_typeof(log(2.5, 10.9::float));
 ----
-2.606992 Float32
+2.606992159958 Float64
 
 query RT
 SELECT log(2.5, 10.9::double), arrow_typeof(log(2.5, 10.9::double));
 ----
 2.606992198152 Float64
 
+# Homogeneous float widths are preserved
+query RT
+SELECT log(arrow_cast(2.5, 'Float16'), arrow_cast(10.9, 'Float16')), arrow_typeof(log(arrow_cast(2.5, 'Float16'), arrow_cast(10.9, 'Float16')));
+----
+2.6074219 Float16
+
+query RT
+SELECT log(2.5::float, 10.9::float), arrow_typeof(log(2.5::float, 10.9::float));
+----
+2.606992 Float32
+
+# #22581: a Float64 base must keep full precision against a Float32 value.
+# 16777217 is not representable in Float32, so an f32 computation would round it.
+query RT
+SELECT log(arrow_cast(16777217, 'Float64'), arrow_cast(2.0, 'Float32')), arrow_typeof(log(arrow_cast(16777217, 'Float64'), arrow_cast(2.0, 'Float32')));
+----
+0.041666666517 Float64
+
+# A NULL value takes the resolved (widest concrete) float type
+query RT
+SELECT log(2.5::float, NULL), arrow_typeof(log(2.5::float, NULL));
+----
+NULL Float32
+
+# #22581: the log(base, 1) simplification must carry the resolved (widest)
+# type, or a Float16 base with a wider literal 1 fails the optimizer's
+# schema-compatibility check at planning time.
+query RT
+SELECT log(arrow_cast(2.5, 'Float16'), 1.0::double), arrow_typeof(log(arrow_cast(2.5, 'Float16'), 1.0::double));
+----
+0 Float64
+
+query RT
+SELECT log(arrow_cast(2.5, 'Float16'), arrow_cast(1.0, 'Float16')), arrow_typeof(log(arrow_cast(2.5, 'Float16'), arrow_cast(1.0, 'Float16')));
+----
+0 Float16
+
 # lcm with array and scalar
 
 query I

datafusion/sqllogictest/test_files/scalar.slt

@@ -694,8 +694,8 @@ select log(2,arrow_cast(a, 'Float64')), log(4,arrow_cast(b, 'Float32')) from sig
 ----
 1 NaN
 2 NULL
-NaN 3.321928
-NaN 6.643856
+NaN 3.321928094887
+NaN 6.643856189775
 
 
 ## log10