You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "1fanwang (via GitHub)" <gi...@apache.org> on 2026/09/09 23:32:28 UTC
[PR] fix: wrong log and power results for a base of 1, 0 or negative [datafusion]
1fanwang opened a new pull request, #25138:
URL: https://github.com/apache/datafusion/pull/25138
## Which issue does this PR close?
- Closes #25137.
## Rationale for this change
`power(1.0, log(1.0, b))` returns `b` instead of `1.0`, and a filter over it
matches rows it should not:
```sql
SELECT count(*) FROM t WHERE power(1.0, log(1.0, b)) = 2.0;
-- returns 1, should be 0
```
Four rewrites assume the logarithm and power identity holds for their shared
base, which is only true for a base that is positive and not 1. They checked
nullability, which says nothing about the value, so a base of 1, 0, negative,
NaN or infinity folded wrongly. The same query over a nullable column already
returned the correct result, so the answer depended on the schema, not the data.
## What changes are included in this PR?
The four rewrites now fire only when the base is a literal that is finite,
positive and not 1. A base whose value cannot be proven, such as a column,
keeps the original expression rather than folding.
## What is the testing strategy for this PR?
`test_log_simplify_rejects_degenerate_base` covers all three log rewrites
against bases of 1, 0, -2, NaN and infinity, and `test_log_simplify_accepts_valid_base`
confirms a valid base still folds. The issue's reproducer was also run through
`datafusion-cli` on the merge base and on this branch.
<details><summary>Reproducer, before and after</summary>
```sql
CREATE VIEW t AS SELECT * FROM (VALUES (2.0), (3.0)) s(b);
SELECT b, power(1.0, log(1.0, b)) AS p, log(1.0, power(1.0, b)) AS l FROM t;
SELECT count(*) AS c FROM t WHERE power(1.0, log(1.0, b)) = 2.0;
CREATE VIEW nn AS SELECT * FROM (VALUES (1.0), (0.0), (-2.0), (2.0)) s(a);
SELECT a, log(a, 1.0) AS l1, log(a, a) AS la FROM nn;
```
Before, on the merge base:
```
| b | p | l | | c | | a | l1 | la |
| 2.0 | 2.0 | 2.0 | | 1 | | 1.0 | 0.0 | 1.0 |
| 3.0 | 3.0 | 3.0 | | 0.0 | 0.0 | 1.0 |
| -2.0 | 0.0 | 1.0 |
| 2.0 | 0.0 | 1.0 |
```
After, on this branch:
```
| b | p | l | | c | | a | l1 | la |
| 2.0 | 1.0 | NaN | | 0 | | 1.0 | NaN | NaN |
| 3.0 | 1.0 | NaN | | 0.0 | -0.0 | NaN |
| -2.0 | NaN | NaN |
| 2.0 | 0.0 | 1.0 |
```
</details>
## Are there any user-facing changes?
Yes. Queries whose base is a literal 1, 0, negative, NaN or infinity now return
the correct value instead of a folded one. No API change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: wrong log and power results for invalid bases and inputs [datafusion]
Posted by "1fanwang (via GitHub)" <gi...@apache.org>.
1fanwang commented on PR #25138:
URL: https://github.com/apache/datafusion/pull/25138#issuecomment-5808015492
Done in https://github.com/apache/datafusion/commit/0efbdb82e78de223e14fd8cb0c687bea4793f8d3.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: preserve nested log and power results [datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 merged PR #25138:
URL: https://github.com/apache/datafusion/pull/25138
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: wrong log and power results for invalid bases and inputs [datafusion]
Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on PR #25138:
URL: https://github.com/apache/datafusion/pull/25138#issuecomment-5814361598
`log(b, power(b, x)) → x` still disagrees with execution when `power` overflows or underflows, even for a valid literal base:
```sql
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);
-- 2000 Infinity
-- -2000 -Infinity
```
The other two rules (`log(lit, 1)`, `log(lit, lit)`) can't fire after type coercion, because `ConstEvaluator` folds them first (`EXPLAIN SELECT log(2.0, 1.0), log(2.0, 2.0)` already shows `Float64(0)`, `Float64(1)`). I'd drop all three rules and `is_valid_log_base`, same as the `power`/`log` rule you removed. The simplify body then reduces to the negative-scale decimal check plus `Ok(ExprSimplifyResult::Original(args))`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: wrong log and power results for a base of 1, 0 or negative [datafusion]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #25138:
URL: https://github.com/apache/datafusion/pull/25138#issuecomment-5612690020
## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/25138?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `88.46154%` with `6 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 81.80%. Comparing base ([`4048898`](https://app.codecov.io/gh/apache/datafusion/commit/40488988ad596c9b093ad60e1453430d803ce33c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`75d26e4`](https://app.codecov.io/gh/apache/datafusion/commit/75d26e482d57e9ba50e9d040191c82e255bb3996?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 3 commits behind head on main.
| [Files with missing lines](https://app.codecov.io/gh/apache/datafusion/pull/25138?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [datafusion/functions/src/math/log.rs](https://app.codecov.io/gh/apache/datafusion/pull/25138?src=pr&el=tree&filepath=datafusion%2Ffunctions%2Fsrc%2Fmath%2Flog.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9mdW5jdGlvbnMvc3JjL21hdGgvbG9nLnJz) | 88.23% | [0 Missing and 6 partials :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/25138?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #25138 +/- ##
========================================
Coverage 81.80% 81.80%
========================================
Files 1130 1130
Lines 417754 417889 +135
Branches 417754 417889 +135
========================================
+ Hits 341754 341868 +114
- Misses 55875 55884 +9
- Partials 20125 20137 +12
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/25138?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: preserve nested log and power results [datafusion]
Posted by "1fanwang (via GitHub)" <gi...@apache.org>.
1fanwang commented on PR #25138:
URL: https://github.com/apache/datafusion/pull/25138#issuecomment-5817670773
Done in https://github.com/apache/datafusion/commit/67f13713503e0a1d5bc26370532b7542a83ad730.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org