You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/09/25 02:42:46 UTC
(datafusion) 01/01: fix: preserve nested log and power results (#25138)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-25138-92559afc05431560b93bb8ad85871d7f014a9dd1
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 696eaf58d436faa6532aa0f5875b5a120b94d0eb
Author: Stefan Wang <1f...@gmail.com>
AuthorDate: Fri Sep 25 02:42:22 2026 +0000
fix: preserve nested log and power results (#25138)
## Which issue does this PR close?
Closes https://github.com/apache/datafusion/issues/25137.
## Rationale for this change
For column exponents of 2000 and -2000, a log-of-power query returned
the exponents instead of positive and negative infinity. The query now
preserves the evaluated results.
## What changes are included in this PR?
Remove the logarithm identities and inverse power/log cancellation.
Literal calls still fold through constant evaluation. Null propagation,
negative-scale decimal handling and zero/one-exponent power rules remain
unchanged.
## What is the testing strategy for this PR?
The SQL regression compares nested log/power with explicit evaluation.
These CLI checks ran on macOS arm64 with Rust 1.98.1:
```sh
cargo run --locked --profile ci -j6 --target-dir target -p datafusion-cli -- --quiet --rc -c "SELECT log(2.0, power(2.0, a)), ln(power(2.0, a)) / ln(2.0) FROM (VALUES (2000.0::double), (-2000.0::double)) t(a);"
DATAFUSION_EXPLAIN_FORMAT=indent cargo run --locked --profile ci -j6 --target-dir target -p datafusion-cli -- --quiet --rc -c "EXPLAIN SELECT log(2.0, 1.0) AS zero, log(2.0, 2.0) AS one;"
```
<details><summary>Raw logs</summary>
Before, on
https://github.com/apache/datafusion/commit/0661d606ccfede2e0b0ba33466e15fea7c45089c:
```text
| 2000.0 | inf |
| -2000.0 | -inf |
```
After, the same query through the CLI:
```text
| inf | inf |
| -inf | -inf |
cli_exit_code=0
```
Literal calls still fold:
```text
| logical_plan | Projection: Float64(0) AS zero, Float64(1) AS one |
| physical_plan | ProjectionExec: expr=[0 as zero, 1 as one] |
```
</details>
## Are there any user-facing changes?
These are result corrections. There is no public API change.
---------
Signed-off-by: 1fanwang <1f...@gmail.com>
---
datafusion/core/tests/expr_api/simplification.rs | 46 ++++++-----
datafusion/functions/src/math/log.rs | 101 ++++-------------------
datafusion/functions/src/math/power.rs | 30 +------
datafusion/sqllogictest/test_files/math.slt | 36 +++++++-
4 files changed, 77 insertions(+), 136 deletions(-)
diff --git a/datafusion/core/tests/expr_api/simplification.rs b/datafusion/core/tests/expr_api/simplification.rs
index e9a975239a..64ff1be99f 100644
--- a/datafusion/core/tests/expr_api/simplification.rs
+++ b/datafusion/core/tests/expr_api/simplification.rs
@@ -606,24 +606,29 @@ fn test_simplify_with_cycle_count(
#[test]
fn test_simplify_log() {
- // Log(c3, 1) ===> 0
{
- let expr = log(col("c3_non_null"), lit(1));
- test_simplify(expr, lit(0i64));
+ let expr = log(lit(2.0), lit(1.0));
+ test_simplify(expr, lit(0.0));
}
- // Log(c3, c3) ===> 1
{
- let expr = log(col("c3_non_null"), col("c3_non_null"));
- let expected = lit(1i64);
+ let expr = log(lit(2.0), lit(2.0));
+ let expected = lit(1.0);
test_simplify(expr, expected);
}
- // Log(c3, Power(c3, c4)) ===> c4
{
- let expr = log(
- col("c3_non_null"),
- power(col("c3_non_null"), col("c4_non_null")),
- );
- let expected = col("c4_non_null");
+ let expr = log(lit(2i64), power(lit(2i64), col("c4_non_null")));
+ test_simplify(expr.clone(), expr);
+ }
+ // Log(c3, 1) ===> Log(c3, 1), since c3 may be 1, 0 or negative
+ {
+ let expr = log(col("c3_non_null"), lit(1));
+ let expected = log(col("c3_non_null"), lit(1));
+ test_simplify(expr, expected);
+ }
+ // Log(c3, c3) ===> Log(c3, c3)
+ {
+ let expr = log(col("c3_non_null"), col("c3_non_null"));
+ let expected = log(col("c3_non_null"), col("c3_non_null"));
test_simplify(expr, expected);
}
// Log(c3, c4) ===> Log(c3, c4)
@@ -649,21 +654,22 @@ fn test_simplify_power() {
Expr::Cast(Cast::new(Box::new(col("c3_non_null")), DataType::Float64));
test_simplify(expr, expected)
}
- // Power(c3, Log(c3, c4)) ===> cast(c4 AS Float64)
- // The simplifier rewrites `power(b, log(b, x))` to `x`, but the
- // rewritten expression must keep the same type as the original
- // `power` call. `power` returns Float64, so the UInt32 c4 has to be cast
- // to Float64 to preserve the output schema the optimizer already
- // committed to.
+ // Power(c3, Log(c3, c4)) is left alone, since c3 may be 1, 0 or negative.
{
let expr = power(
col("c3_non_null"),
log(col("c3_non_null"), col("c4_non_null")),
);
- let expected =
- Expr::Cast(Cast::new(Box::new(col("c4_non_null")), DataType::Float64));
+ let expected = power(
+ col("c3_non_null"),
+ log(col("c3_non_null"), col("c4_non_null")),
+ );
test_simplify(expr, expected)
}
+ {
+ let expr = power(lit(2i64), log(lit(2i64), col("c4_non_null")));
+ test_simplify(expr.clone(), expr)
+ }
// Power(c3, c4) ===> Power(c3, c4)
{
let expr = power(col("c3_non_null"), col("c4_non_null"));
diff --git a/datafusion/functions/src/math/log.rs b/datafusion/functions/src/math/log.rs
index 77a730808d..5538181f1c 100644
--- a/datafusion/functions/src/math/log.rs
+++ b/datafusion/functions/src/math/log.rs
@@ -17,8 +17,6 @@
//! Math function: `log()`.
-use super::power::PowerFunc;
-
use crate::utils::calculate_binary_math;
use arrow::array::{Array, ArrayRef};
use arrow::datatypes::{
@@ -28,15 +26,12 @@ use arrow::datatypes::{
use arrow::error::ArrowError;
use arrow_buffer::i256;
use datafusion_common::types::NativeType;
-use datafusion_common::{
- Result, ScalarValue, exec_err, internal_err, plan_datafusion_err, plan_err,
-};
-use datafusion_expr::expr::ScalarFunction;
+use datafusion_common::{Result, ScalarValue, exec_err, plan_datafusion_err, plan_err};
use datafusion_expr::simplify::{ExprSimplifyResult, SimplifyContext};
use datafusion_expr::sort_properties::{ExprProperties, SortProperties};
use datafusion_expr::{
- Coercion, ColumnarValue, Documentation, Expr, ScalarFunctionArgs, ScalarUDF,
- TypeSignature, TypeSignatureClass, lit,
+ Coercion, ColumnarValue, Documentation, Expr, ScalarFunctionArgs, TypeSignature,
+ TypeSignatureClass, lit,
};
use datafusion_expr::{ScalarUDFImpl, Signature, Volatility};
use datafusion_macros::user_doc;
@@ -312,16 +307,12 @@ impl ScalarUDFImpl for LogFunc {
self.doc()
}
- /// Simplify the `log` function by the relevant rules:
- /// 1. Log(a, 1) ===> 0
- /// 2. Log(a, Power(a, b)) ===> b
- /// 3. Log(a, a) ===> 1
fn simplify(
&self,
- mut args: Vec<Expr>,
+ args: Vec<Expr>,
info: &SimplifyContext,
) -> Result<ExprSimplifyResult> {
- let mut arg_types = args
+ let arg_types = args
.iter()
.map(|arg| info.get_data_type(arg))
.collect::<Result<Vec<_>>>()?;
@@ -354,60 +345,10 @@ 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 base_nullable = info.nullable(&base)?;
-
- match number {
- Expr::Literal(value, _)
- if value == ScalarValue::new_one(&number_datatype)? && !base_nullable =>
- {
- 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]
- && !base_nullable =>
- {
- let b = args.pop().unwrap(); // length checked above
- Ok(ExprSimplifyResult::Simplified(b))
- }
- number => {
- if number == base && !base_nullable {
- 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(args))
}
}
-/// Returns true if the function is `PowerFunc`
-fn is_pow(func: &ScalarUDF) -> bool {
- func.inner().is::<PowerFunc>()
-}
-
#[cfg(test)]
mod tests {
use std::sync::Arc;
@@ -766,26 +707,20 @@ mod tests {
}
#[test]
- // Test that non-simplifiable log() expressions are unchanged after simplification
fn test_log_simplify_original() {
let context = SimplifyContext::default();
- // One argument with no simplifications
- let result = LogFunc::new().simplify(vec![lit(2)], &context).unwrap();
- let ExprSimplifyResult::Original(args) = result else {
- panic!("Expected ExprSimplifyResult::Original")
- };
- assert_eq!(args.len(), 1);
- assert_eq!(args[0], lit(2));
- // Two arguments with no simplifications
- let result = LogFunc::new()
- .simplify(vec![lit(2), lit(3)], &context)
- .unwrap();
- let ExprSimplifyResult::Original(args) = result else {
- panic!("Expected ExprSimplifyResult::Original")
- };
- assert_eq!(args.len(), 2);
- assert_eq!(args[0], lit(2));
- assert_eq!(args[1], lit(3));
+ for args in [
+ vec![lit(2)],
+ vec![lit(2), lit(3)],
+ vec![lit(2.0), lit(1.0)],
+ vec![lit(2.0), lit(2.0)],
+ ] {
+ let result = LogFunc::new().simplify(args.clone(), &context).unwrap();
+ let ExprSimplifyResult::Original(actual) = result else {
+ panic!("Expected ExprSimplifyResult::Original")
+ };
+ assert_eq!(actual, args);
+ }
}
#[test]
diff --git a/datafusion/functions/src/math/power.rs b/datafusion/functions/src/math/power.rs
index 30ac401b5f..d63ef2fa71 100644
--- a/datafusion/functions/src/math/power.rs
+++ b/datafusion/functions/src/math/power.rs
@@ -16,8 +16,6 @@
// under the License.
//! Math function: `power()`.
-use super::log::LogFunc;
-
use crate::utils::calculate_binary_math;
use arrow::array::{Array, ArrayRef};
use arrow::datatypes::{DataType, Float64Type};
@@ -25,10 +23,9 @@ use arrow::error::ArrowError;
use datafusion_common::types::{NativeType, logical_float64};
use datafusion_common::utils::take_function_args;
use datafusion_common::{Result, ScalarValue, internal_err};
-use datafusion_expr::expr::ScalarFunction;
use datafusion_expr::simplify::{ExprSimplifyResult, SimplifyContext};
use datafusion_expr::{
- Cast, Coercion, ColumnarValue, Documentation, Expr, ScalarFunctionArgs, ScalarUDF,
+ Cast, Coercion, ColumnarValue, Documentation, Expr, ScalarFunctionArgs,
ScalarUDFImpl, Signature, TypeSignatureClass, Volatility, lit,
};
use datafusion_macros::user_doc;
@@ -131,7 +128,6 @@ impl ScalarUDFImpl for PowerFunc {
/// Simplify the `power` function by the relevant rules:
/// 1. Power(a, 0) ===> 1
/// 2. Power(a, 1) ===> a
- /// 3. Power(a, Log(a, b)) ===> b
fn simplify(
&self,
args: Vec<Expr>,
@@ -151,13 +147,6 @@ impl ScalarUDFImpl for PowerFunc {
)));
}
- // `simplify` runs on the logical expression *before* type coercion,
- // so a simplified sub-expression may still carry its original type
- // rather than the Float64 that `power` is declared to return. Cast it
- // back when needed to preserve the schema the optimizer already
- // committed to — e.g. `power(int_col, 1)` simplifies to `int_col`,
- // and the `b` in `power(b, log(b, uint_col))` simplifies to `uint_col`,
- // both of which must become Float64.
let cast_to_return_type = |expr: Expr, expr_type: &DataType| {
if expr_type == &return_type {
expr
@@ -179,18 +168,6 @@ impl ScalarUDFImpl for PowerFunc {
base, &base_type,
)))
}
- Expr::ScalarFunction(ScalarFunction { func, mut args })
- if is_log(&func)
- && args.len() == 2
- && base == args[0]
- && !base_nullable =>
- {
- let b = args.pop().unwrap(); // length checked above
- let b_type = info.get_data_type(&b)?;
- Ok(ExprSimplifyResult::Simplified(cast_to_return_type(
- b, &b_type,
- )))
- }
_ => Ok(ExprSimplifyResult::Original(vec![base, exponent])),
}
}
@@ -200,11 +177,6 @@ impl ScalarUDFImpl for PowerFunc {
}
}
-/// Return true if this function call is a call to `Log`
-fn is_log(func: &ScalarUDF) -> bool {
- func.inner().is::<LogFunc>()
-}
-
#[cfg(test)]
mod tests {
use super::*;
diff --git a/datafusion/sqllogictest/test_files/math.slt b/datafusion/sqllogictest/test_files/math.slt
index 0613259787..35dd69618b 100644
--- a/datafusion/sqllogictest/test_files/math.slt
+++ b/datafusion/sqllogictest/test_files/math.slt
@@ -1213,7 +1213,9 @@ physical_plan
01)ProjectionExec: expr=[log(column1@0, 1) as l1, log(column1@0, column1@0) as la, power(column1@0, 0) as p0, power(column1@0, log(column1@0, column2@1)) as pl]
02)--DataSourceExec: partitions=1, partition_sizes=[1]
-# Non-nullable bases still use the existing simplifications
+# A column base is left alone even when it is not nullable, because its value is
+# not known to be outside {0, 1, negative}. power(a, 0) does not depend on the
+# base value, so it still folds.
query TT
EXPLAIN SELECT
log(a, 1) AS l1,
@@ -1223,14 +1225,40 @@ EXPLAIN SELECT
FROM (VALUES (2.0::double, 3.0::double)) AS t(a, b);
----
logical_plan
-01)Projection: Float64(0) AS l1, Float64(1) AS la, Float64(1) AS p0, t.b AS pl
+01)Projection: log(t.a, Float64(1)) AS l1, log(t.a, t.a) AS la, Float64(1) AS p0, power(t.a, log(t.a, t.b)) AS pl
02)--SubqueryAlias: t
-03)----Projection: column2 AS b
+03)----Projection: column1 AS a, column2 AS b
04)------Values: (Float64(2), Float64(3))
physical_plan
-01)ProjectionExec: expr=[0 as l1, 1 as la, 1 as p0, column2@1 as pl]
+01)ProjectionExec: expr=[log(column1@0, 1) as l1, log(column1@0, column1@0) as la, 1 as p0, power(column1@0, log(column1@0, column2@1)) as pl]
02)--DataSourceExec: partitions=1, partition_sizes=[1]
+query RR
+SELECT power(2.0, log(2.0, a)), power(b, log(b, a))
+FROM (VALUES (-3.0::double, 2.0::double)) t(a, b);
+----
+NaN NaN
+
+query I
+SELECT count(*) FROM (VALUES (-3.0::double)) t(a)
+WHERE power(2.0, log(2.0, a)) = -3.0;
+----
+0
+
+query R rowsort
+SELECT power(2.0, log(2.0, a))
+FROM (VALUES (4.0::double), (NULL::double)) t(a);
+----
+4
+NULL
+
+query RR
+SELECT log(2.0, power(2.0, a)), ln(power(2.0, a)) / ln(2.0)
+FROM (VALUES (2000.0::double), (-2000.0::double)) t(a);
+----
+Infinity Infinity
+-Infinity -Infinity
+
# Float 16/32/64 for log
query RT
SELECT log(2.5, arrow_cast(10.9, 'Float16')), arrow_typeof(log(2.5, arrow_cast(10.9, 'Float16')));
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org