Fix log(0.0::float8) should error, not return -inf

Author: xiedeyantu

datafusion/functions/src/math/log.rs

@@ -106,6 +106,17 @@ fn is_valid_integer_base(base: f64) -> bool {
     base.trunc() == base && base >= 2.0 && base <= u32::MAX as f64
 }
 
+#[inline]
+fn validate_log_value(value: f64) -> Result<(), ArrowError> {
+    if value == 0.0 {
+        Err(ArrowError::ComputeError(
+            "cannot take logarithm of zero".to_string(),
+        ))
+    } else {
+        Ok(())
+    }
+}
+
 /// Calculate logarithm for Decimal32 values.
 /// For integer bases >= 2 with zero scale, return an exact integer log when the
 /// value is a perfect power of the base. Otherwise falls back to f64 computation.
@@ -121,7 +132,10 @@ fn log_decimal32(value: i32, scale: i8, base: f64) -> Result<f64, ArrowError> {
             return Ok(int_log as f64);
         }
     }
-    decimal_to_f64(value, scale).map(|v| v.log(base))
+    decimal_to_f64(value, scale).and_then(|v| {
+        validate_log_value(v)?;
+        Ok(v.log(base))
+    })
 }
 
 /// Calculate logarithm for Decimal64 values.
@@ -139,7 +153,10 @@ fn log_decimal64(value: i64, scale: i8, base: f64) -> Result<f64, ArrowError> {
             return Ok(int_log as f64);
         }
     }
-    decimal_to_f64(value, scale).map(|v| v.log(base))
+    decimal_to_f64(value, scale).and_then(|v| {
+        validate_log_value(v)?;
+        Ok(v.log(base))
+    })
 }
 
 /// Calculate logarithm for Decimal128 values.
@@ -157,7 +174,10 @@ fn log_decimal128(value: i128, scale: i8, base: f64) -> Result<f64, ArrowError>
             return Ok(int_log as f64);
         }
     }
-    decimal_to_f64(value, scale).map(|v| v.log(base))
+    decimal_to_f64(value, scale).and_then(|v| {
+        validate_log_value(v)?;
+        Ok(v.log(base))
+    })
 }
 
 /// Convert a scaled decimal value to f64.
@@ -180,7 +200,9 @@ fn log_decimal256(value: i256, scale: i8, base: f64) -> Result<f64, ArrowError>
                 ArrowError::ComputeError(format!("Cannot convert {value} to f64"))
             })?;
             let scale_factor = 10f64.powi(scale as i32);
-            Ok((value_f64 / scale_factor).log(base))
+            let value = value_f64 / scale_factor;
+            validate_log_value(value)?;
+            Ok(value.log(base))
         }
     }
 }
@@ -247,27 +269,33 @@ 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 => calculate_binary_math::<
+                Float16Type,
+                Float16Type,
+                Float16Type,
+                _,
+            >(&value, &base, |value, base| {
+                validate_log_value(value.to_f64())?;
+                Ok(value.log(base))
+            })?,
+            DataType::Float32 => calculate_binary_math::<
+                Float32Type,
+                Float32Type,
+                Float32Type,
+                _,
+            >(&value, &base, |value, base| {
+                validate_log_value(value as f64)?;
+                Ok(value.log(base))
+            })?,
+            DataType::Float64 => calculate_binary_math::<
+                Float64Type,
+                Float64Type,
+                Float64Type,
+                _,
+            >(&value, &base, |value, base| {
+                validate_log_value(value)?;
+                Ok(value.log(base))
+            })?,
             DataType::Decimal32(_, scale) => {
                 calculate_binary_math::<Decimal32Type, Float64Type, Float64Type, _>(
                     &value,

datafusion/sqllogictest/test_files/scalar.slt

@@ -625,10 +625,8 @@ select log(2, 2.0/3) a, log(10, 2.0/3) b;
 
 # log scalar ops with zero edgecases
 # please see https://github.com/apache/datafusion/pull/5245#issuecomment-1426828382
-query RR rowsort
-select log(0) a, log(1, 64) b;
-----
--Infinity Infinity
+query error cannot take logarithm of zero
+select log(0) a;
 
 # log with columns #1
 query RRR rowsort