U0001F3A2 opened a new pull request, #23676: URL: https://github.com/apache/datafusion/pull/23676
## Which issue does this PR close? - Closes #22581. ## Rationale for this change `log(base, value)` resolved its result type and computed only from the value (the last argument). When the base was a wider float than the value, `calculate_binary_math` narrowed the base to the value's type, so the whole computation happened in the narrower float and lost precision: ```sql SELECT log(arrow_cast(16777217,'Float64'), arrow_cast(2.0,'Float32')); -- 0.041666668 (Float32) SELECT log(arrow_cast(16777217,'Float64'), arrow_cast(2.0,'Float64')); -- 0.041666666517376175 (Float64) ``` 16777217 is not representable in f32, so narrowing the base to f32 rounds it. The reverse argument order already returned Float64 because the value drove the type. ## What changes are included in this PR? - `return_type` now resolves to the widest float among all arguments instead of keying off the value alone. - `invoke_with_args` computes in that widest float, widening the value to match rather than narrowing the base. The key detail: `calculate_binary_math` only casts the base, so the value array is cast up first. - Integer/decimal arguments already coerce to Float64; null arguments do not constrain the width. Homogeneous float widths and the decimal-value paths are unchanged. The existing `math.slt` cases that asserted the narrowed type for `log(Float64 base, narrower value)` are updated, since they encoded the bug. Homogeneous-width coverage is kept. One small consequence worth noting: `log(Float32, NULL)` now reports Float32 instead of Float64 (the result is still NULL), matching the resolved type of the one concrete argument. ## Are these changes tested? Yes. - Unit tests in `log.rs`: the mixed-width matrix via `return_type` (both orders, Float16, NULL), and a precision test on the `16777217` case asserting a Float64 result that does not match the lossy f32 value. Both verified to fail on `main`. - `math.slt`: the `#22581` repro plus mixed and homogeneous width cases. `cargo test -p datafusion-functions --lib`, the `math` sqllogictests, `cargo fmt`, and `cargo clippy` are clean. ## Are there any user-facing changes? Yes. `log(base, value)` where the base is a wider float now returns and computes in that wider float (e.g. `log(Float64, Float32)` returns Float64), fixing the precision loss. 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
