Commit cbf33d1
Fix regression for negative-scale decimal128 in log (#19315)
## Which issue does this PR close?
- Part of #19250
## Rationale for this change
Previously, the `log` function would fail when operating on decimal
values with negative scales.
Negative scales in decimals represent values where the scale indicates
padding zeros to the right (e.g., `Decimal128(38, -2)` with value `100`
represents `10000`). This PR restores support for negative-scale
decimals in the `log` function by implementing the logarithmic property:
`log_base(value * 10^(-scale)) = log_base(value) + (-scale) *
log_base(10)`.
## What changes are included in this PR?
1. **Enhanced `log_decimal128` function**:
- Added support for negative scales using the logarithmic property
- For negative scales, computes `log_base(value) + (-scale) *
log_base(10)` instead of trying to convert to unscaled value
- Added detection for negative-scale decimals in both the number and
base arguments
- Skips simplification when negative scales are detected to avoid errors
with `ScalarValue` (which doesn't support negative scales yet)
2. **Added comprehensive tests**:
- Unit tests in `log.rs` for negative-scale decimals with various bases
(2, 3, 10)
- SQL logic tests in `decimal.slt` using scientific notation (e.g.,
`1e4`, `8e1`) to create decimals with negative scales
## Are these changes tested?
Yes, this PR includes comprehensive tests:
1. Unit tests:
- `test_log_decimal128_negative_scale`: Tests array inputs with negative
scales
- `test_log_decimal128_negative_scale_base2`: Tests with base 2 and
negative scales
- `test_log_decimal128_negative_scale_scalar`: Tests scalar inputs with
negative scales
2. SQL logic tests:
- Tests for unary log with negative scales (`log(1e4)`)
- Tests for binary log with explicit base 10 (`log(10, 1e4)`)
- Tests for binary log with base 2 (`log(2.0, 8e1)`, `log(2.0, 16e1)`)
- Tests for different negative scale values (`log(5e3)`)
- Tests for array operations with negative scales
- Tests for different bases (2, 3, 10) with negative-scale decimals
All tests pass successfully.
## Are there any user-facing changes?
Yes, this is a user-facing change:
**Before**: The `log` function would fail with an error when operating
on decimal values with negative scales:
```sql
-- This would fail
SELECT log(1e4); -- Error: Negative scale is not supported
```
**After**: The `log` function now correctly handles decimal values with
negative scales:
```sql
-- This now works
SELECT log(1e4); -- Returns 4.0 (log10(10000))
SELECT log(2.0, 8e1); -- Returns ~6.32 (log2(80))
```
---------
Co-authored-by: Jeffrey Vo <[email protected]>
Co-authored-by: Martin Grigorov <[email protected]>1 parent c2747eb commit cbf33d1
File tree
2 files changed
+74
-5
lines changed- datafusion
- functions/src/math
- sqllogictest/test_files
2 files changed
+74
-5
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
166 | 166 | | |
167 | 167 | | |
168 | 168 | | |
169 | | - | |
170 | | - | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
171 | 179 | | |
172 | 180 | | |
173 | | - | |
174 | | - | |
175 | | - | |
176 | 181 | | |
177 | 182 | | |
178 | 183 | | |
| |||
342 | 347 | | |
343 | 348 | | |
344 | 349 | | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
345 | 363 | | |
346 | 364 | | |
347 | 365 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
918 | 918 | | |
919 | 919 | | |
920 | 920 | | |
| 921 | + | |
| 922 | + | |
| 923 | + | |
| 924 | + | |
| 925 | + | |
| 926 | + | |
| 927 | + | |
| 928 | + | |
| 929 | + | |
| 930 | + | |
| 931 | + | |
| 932 | + | |
| 933 | + | |
| 934 | + | |
| 935 | + | |
| 936 | + | |
| 937 | + | |
| 938 | + | |
| 939 | + | |
| 940 | + | |
| 941 | + | |
| 942 | + | |
| 943 | + | |
| 944 | + | |
| 945 | + | |
| 946 | + | |
| 947 | + | |
| 948 | + | |
| 949 | + | |
| 950 | + | |
| 951 | + | |
| 952 | + | |
| 953 | + | |
| 954 | + | |
| 955 | + | |
| 956 | + | |
| 957 | + | |
| 958 | + | |
| 959 | + | |
| 960 | + | |
| 961 | + | |
| 962 | + | |
| 963 | + | |
| 964 | + | |
| 965 | + | |
| 966 | + | |
| 967 | + | |
| 968 | + | |
| 969 | + | |
| 970 | + | |
| 971 | + | |
921 | 972 | | |
922 | 973 | | |
923 | 974 | | |
| |||
0 commit comments