From 3e6d538ab884bcf2c71536e933edddcab38068b7 Mon Sep 17 00:00:00 2001 From: Nishchaya Sharma <129059557+shinzoxD@users.noreply.github.com> Date: Sun, 16 Aug 2026 01:52:44 +0530 Subject: [PATCH] fix: error on log of zero instead of returning -inf PostgreSQL treats log(0.0::float8) as a domain error. DataFusion previously returned -inf via IEEE 754. Match the same class of domain error already used for sqrt(negative), power(0, negative), and factorial(negative). Only the logged value is checked. A zero base (log(0, x)) and log(1, 64) are left unchanged so this stays scoped to #22261. --- datafusion/functions/src/math/log.rs | 203 ++++++++++++++++-- datafusion/sqllogictest/test_files/math.slt | 20 ++ datafusion/sqllogictest/test_files/scalar.slt | 11 +- docs/source/user-guide/expressions.md | 19 +- 4 files changed, 218 insertions(+), 35 deletions(-) diff --git a/datafusion/functions/src/math/log.rs b/datafusion/functions/src/math/log.rs index 732cfff6cf053..a10198f85ede9 100644 --- a/datafusion/functions/src/math/log.rs +++ b/datafusion/functions/src/math/log.rs @@ -100,6 +100,27 @@ impl LogFunc { } } +/// Matches PostgreSQL: `log(0::float8)` is undefined (IEEE 754 would yield -inf). +const LOG_OF_ZERO_ERROR: &str = "cannot take logarithm of zero"; + +#[inline] +fn log_of_zero_error() -> ArrowError { + ArrowError::ComputeError(LOG_OF_ZERO_ERROR.to_string()) +} + +/// Compute `value.log(base)` after rejecting a zero value. +/// +/// Only the value (the number being logged) is checked. A zero *base* +/// (`log(0, x)`) is left as IEEE 754 infinity / NaN; that is a separate +/// compatibility case from issue #22261. +#[inline] +fn compute_float_log(value: F, base: F) -> Result { + if value == F::zero() { + return Err(log_of_zero_error()); + } + Ok(value.log(base)) +} + /// Checks if the base is valid for the efficient integer logarithm algorithm. #[inline] fn is_valid_integer_base(base: f64) -> bool { @@ -110,6 +131,9 @@ fn is_valid_integer_base(base: f64) -> bool { /// 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. fn log_decimal32(value: i32, scale: i8, base: f64) -> Result { + if value == 0 { + return Err(log_of_zero_error()); + } if scale == 0 && is_valid_integer_base(base) && let Ok(unscaled) = u32::try_from(value) @@ -128,6 +152,9 @@ fn log_decimal32(value: i32, scale: i8, base: f64) -> Result { /// 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. fn log_decimal64(value: i64, scale: i8, base: f64) -> Result { + if value == 0 { + return Err(log_of_zero_error()); + } if scale == 0 && is_valid_integer_base(base) && let Ok(unscaled) = u64::try_from(value) @@ -146,6 +173,9 @@ fn log_decimal64(value: i64, scale: i8, base: f64) -> Result { /// 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. fn log_decimal128(value: i128, scale: i8, base: f64) -> Result { + if value == 0 { + return Err(log_of_zero_error()); + } if scale == 0 && is_valid_integer_base(base) && let Ok(unscaled) = u128::try_from(value) @@ -171,6 +201,9 @@ fn decimal_to_f64(value: T, scale: i8) -> Result Result { + if value == i256::ZERO { + return Err(log_of_zero_error()); + } // Try to convert to i128 for the optimized path match value.to_i128() { Some(v) => log_decimal128(v, scale, base), @@ -251,27 +284,24 @@ impl ScalarUDFImpl for LogFunc { let value = value.to_array(args.number_rows)?; let output: ArrayRef = match value.data_type() { - DataType::Float16 => { - calculate_binary_math::( - &value, - &base, - |value, base| Ok(value.log(base)), - )? - } - DataType::Float32 => { - calculate_binary_math::( - &value, - &base, - |value, base| Ok(value.log(base)), - )? - } - DataType::Float64 => { - calculate_binary_math::( - &value, - &base, - |value, base| Ok(value.log(base)), - )? - } + DataType::Float16 => calculate_binary_math::< + Float16Type, + Float16Type, + Float16Type, + _, + >(&value, &base, compute_float_log)?, + DataType::Float32 => calculate_binary_math::< + Float32Type, + Float32Type, + Float32Type, + _, + >(&value, &base, compute_float_log)?, + DataType::Float64 => calculate_binary_math::< + Float64Type, + Float64Type, + Float64Type, + _, + >(&value, &base, compute_float_log)?, DataType::Decimal32(_, scale) => { calculate_binary_math::( &value, @@ -1213,4 +1243,135 @@ mod tests { } } } + + fn invoke_log( + args: Vec, + data_types: Vec, + ) -> Result { + let number_rows = args + .iter() + .map(|a| match a { + ColumnarValue::Array(arr) => arr.len(), + ColumnarValue::Scalar(_) => 1, + }) + .max() + .unwrap_or(1); + let arg_fields = data_types + .into_iter() + .map(|dt| Field::new("a", dt, false).into()) + .collect(); + let args = ScalarFunctionArgs { + args, + arg_fields, + number_rows, + return_field: Field::new("f", DataType::Float64, true).into(), + config_options: Arc::new(ConfigOptions::default()), + }; + LogFunc::new().invoke_with_args(args) + } + + fn assert_log_of_zero(err: datafusion_common::DataFusionError) { + let message = err.to_string(); + assert!( + message.contains(LOG_OF_ZERO_ERROR), + "expected '{LOG_OF_ZERO_ERROR}' in error, got {message}" + ); + } + + #[test] + fn test_log_zero_float64_unary_errors() { + let err = invoke_log( + vec![ColumnarValue::Scalar(ScalarValue::Float64(Some(0.0)))], + vec![DataType::Float64], + ) + .expect_err("log(0.0) should be a domain error"); + assert_log_of_zero(err); + } + + #[test] + fn test_log_negative_zero_float64_errors() { + let err = invoke_log( + vec![ColumnarValue::Scalar(ScalarValue::Float64(Some(-0.0)))], + vec![DataType::Float64], + ) + .expect_err("log(-0.0) should be a domain error"); + assert_log_of_zero(err); + } + + #[test] + fn test_log_zero_float32_unary_errors() { + let err = invoke_log( + vec![ColumnarValue::Scalar(ScalarValue::Float32(Some(0.0)))], + vec![DataType::Float32], + ) + .expect_err("log(0.0f32) should be a domain error"); + assert_log_of_zero(err); + } + + #[test] + fn test_log_zero_float64_binary_errors() { + let err = invoke_log( + vec![ + ColumnarValue::Scalar(ScalarValue::Float64(Some(10.0))), + ColumnarValue::Scalar(ScalarValue::Float64(Some(0.0))), + ], + vec![DataType::Float64, DataType::Float64], + ) + .expect_err("log(10, 0.0) should be a domain error"); + assert_log_of_zero(err); + } + + #[test] + fn test_log_zero_array_errors() { + let err = invoke_log( + vec![ColumnarValue::Array(Arc::new(Float64Array::from(vec![ + 10.0, 0.0, 100.0, + ])))], + vec![DataType::Float64], + ) + .expect_err("log() of an array containing 0 should be a domain error"); + assert_log_of_zero(err); + } + + #[test] + fn test_log_zero_decimal128_errors() { + let err = invoke_log( + vec![ColumnarValue::Scalar(ScalarValue::Decimal128( + Some(0), + 38, + 0, + ))], + vec![DataType::Decimal128(38, 0)], + ) + .expect_err("log(0::decimal) should be a domain error"); + assert_log_of_zero(err); + } + + #[test] + fn test_log_zero_decimal256_errors() { + let err = invoke_log( + vec![ColumnarValue::Scalar(ScalarValue::Decimal256( + Some(i256::ZERO), + DECIMAL256_MAX_PRECISION, + 0, + ))], + vec![DataType::Decimal256(DECIMAL256_MAX_PRECISION, 0)], + ) + .expect_err("log(0::decimal256) should be a domain error"); + assert_log_of_zero(err); + } + + #[test] + fn test_log_zero_base_is_not_this_issue() { + // log(0, 64) takes log of 64 (nonzero). A zero *base* is out of + // scope for #22261 and keeps the previous IEEE result. + invoke_log( + vec![ + ColumnarValue::Scalar(ScalarValue::Float64(Some(0.0))), + ColumnarValue::Scalar(ScalarValue::Float64(Some(64.0))), + ], + vec![DataType::Float64, DataType::Float64], + ) + .expect("zero base should not error in this change"); + } } diff --git a/datafusion/sqllogictest/test_files/math.slt b/datafusion/sqllogictest/test_files/math.slt index b6bf51dd4799a..c39c681f1c323 100644 --- a/datafusion/sqllogictest/test_files/math.slt +++ b/datafusion/sqllogictest/test_files/math.slt @@ -1162,6 +1162,26 @@ select ---- 39.0625 Float64 +# log(0::float8) - issue #22261 +query error DataFusion error: Arrow error: Compute error: cannot take logarithm of zero +SELECT log(0.0::float8) + +# two-arg form: logarithm of zero value +query error DataFusion error: Arrow error: Compute error: cannot take logarithm of zero +SELECT log(10.0::float8, 0.0::float8) + +# negative zero is still zero +query error DataFusion error: Arrow error: Compute error: cannot take logarithm of zero +SELECT log((-0.0)::float8) + +# column / non-literal path +query error DataFusion error: Arrow error: Compute error: cannot take logarithm of zero +SELECT log(x) FROM (VALUES (0.0::float8)) AS t(x) + +# decimal zero +query error DataFusion error: Arrow error: Compute error: cannot take logarithm of zero +SELECT log(0::decimal(10, 2)) + # factorial negative (PostgreSQL-compatible domain error) query error DataFusion error: Execution error: factorial of a negative number is undefined select factorial(-1); diff --git a/datafusion/sqllogictest/test_files/scalar.slt b/datafusion/sqllogictest/test_files/scalar.slt index 2bedb28a85962..2e1578dd949d0 100644 --- a/datafusion/sqllogictest/test_files/scalar.slt +++ b/datafusion/sqllogictest/test_files/scalar.slt @@ -644,10 +644,15 @@ 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; +# log(0) is a domain error (PostgreSQL compatibility, #22261) +query error cannot take logarithm of zero +select log(0); + +# log(1, 64) remains IEEE infinity (base-1 is a separate issue) +query R +select log(1, 64); ---- --Infinity Infinity +Infinity # log with columns #1 query RRR rowsort diff --git a/docs/source/user-guide/expressions.md b/docs/source/user-guide/expressions.md index 3fbc11e0c92c0..69e411151d355 100644 --- a/docs/source/user-guide/expressions.md +++ b/docs/source/user-guide/expressions.md @@ -151,17 +151,14 @@ but these operators always return a `bool` which makes them not work with the ex | trunc(x) | truncate toward zero | :::{note} -Unlike to some databases the math functions in Datafusion works the same way as Rust math functions, avoiding failing on corner cases e.g. - -```sql -select log(-1), log(0), sqrt(-1); -+----------------+---------------+-----------------+ -| log(Int64(-1)) | log(Int64(0)) | sqrt(Int64(-1)) | -+----------------+---------------+-----------------+ -| NaN | -inf | NaN | -+----------------+---------------+-----------------+ -``` - +Most math functions in DataFusion follow Rust / IEEE 754 semantics for +corner cases. For example `log(-1)` returns `NaN`. Some domain errors +match PostgreSQL instead and fail the query: + +* `log(0)` / `log(0.0::float8)` — cannot take logarithm of zero +* `sqrt` of a negative number +* `power(0, negative)` +* `factorial` of a negative number ::: ## Conditional Expressions