You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by "vera-rubin (via GitHub)" <gi...@apache.org> on 2026/10/11 23:18:12 UTC

[I] [C++][Python] Safe decimal narrowing to Decimal32 drops coefficient bits before checking [arrow]

vera-rubin opened a new issue, #53288:
URL: https://github.com/apache/arrow/issues/53288

   ### Describe the bug, including details regarding any error messages, version, and platform.
   
   `pc.cast` to Decimal32 can return a different value with default safety enabled. It can also reject an exact scale-changing conversion.
   
   Default safe narrowing can silently change decimal amounts, so downstream calculations may use incorrect values. Downstream application effects were not tested.
   
   Reproduced on CPython 3.12.3, Linux x86_64, glibc 2.39, with the official `pyarrow==26.0.0` wheel on 2026-10-11. PyArrow and Arrow C++ both report `26.0.0`. The wheel is `pyarrow-26.0.0-cp312-cp312-manylinux_2_28_x86_64.whl`, SHA256 `6a628922ba20705fa964ca73e4ef959c2fb2f14b9bbec5589a6a1e68e6257c85`.
   
   Reviewed release source: `apache-arrow-26.0.0` at `446167169a0dc547e00e1bc29f417a40c0ca267a`. The wheel does not expose its build commit. Main `5ef3c5200359bc8e2375daf5f989369b0c8c089b` was read, not built or tested. Its cited kernel file is byte-identical to the release source.
   
   #### Reproduction
   
   The inline listing is an unexecuted shortening of the retained original reproducer and independent probes.
   
   - Original reproducer and run record at `50bf6b998dd2c12ee3c04c6de2d339cd9b84ca3c`: `findings/pyarrow/decimal32-safe-cast/repro.py` and `tasks/pyarrow-b-r01/reproduction-results.json`
   - Independent probes, transcript and environment at `c686294656f1f25e795ba897e4bccbec685aad99`: `findings/pyarrow/verify_independent.py`, `findings/pyarrow/decimal32-safe-cast/VERIFY.md` and `findings/pyarrow/VERIFY-ENVIRONMENT.md`
   
   Run these commands on Linux with Python 3.12.3, save the inline code as `repro.py`, then run the last command:
   
   ```sh
   python3.12 -m venv .venv
   .venv/bin/python -m pip install --index-url https://pypi.org/simple --only-binary=:all: pyarrow==26.0.0
   .venv/bin/python repro.py
   ```
   
   ```python
   from decimal import Decimal
   import pyarrow as pa
   import pyarrow.compute as pc
   
   assert pa.__version__ == pa.cpp_version == "26.0.0"
   cases = [
       ("5000000000", 0, 0),
       ("536870912000", 0, -3),
       ("30000000.00", 2, 0),
   ]
   for text, source_scale, target_scale in cases:
       value = Decimal(text)
       data = pa.array([value], pa.decimal128(12, source_scale))
       data.validate(full=True)
       assert data.to_pylist() == [value]
       assert pc.cast(data, pa.decimal128(15, target_scale)).to_pylist() == [value]
       try:
           got = pc.cast(data, pa.decimal32(9, target_scale)).to_pylist()
       except pa.ArrowInvalid as error:
           got = "ArrowInvalid: " + str(error)
       print(text, got)
   ```
   
   Retained execution excerpts from `findings/pyarrow/decimal32-safe-cast/VERIFY.md`:
   
   Original reproducer (exit 1):
   
   ```text
   PyArrow 26.0.0; C++ 26.0.0
   Failed conformance cases: 96
   AssertionError: Safe decimal32 narrowing violates the independently computed result
   ```
   
   Independent probe:
   
   The independent probe's exit 0 records successful input/control checks while printing disagreements; it does not mean the casts conform.
   
   ```text
   input 5000000000 target coefficient 5000000000 expected overflow error observed [Decimal('705032704')]
   input 536870912000 target coefficient 536870912.000 expected [Decimal('536870912000')] observed [Decimal('0E+3')]
   input 30000000.00 target coefficient 30000000.00 expected [Decimal('30000000.00')] observed ArrowInvalid: Rescaling Decimal value would cause data loss
   PASS: raw/builder agreement, wider rescale, precision-edge and fractional-loss controls
   ```
   
   #### Expected and observed
   
   The [cast contract](https://github.com/apache/arrow/blob/446167169a0dc547e00e1bc29f417a40c0ca267a/python/pyarrow/compute.py#L349-L364) enables overflow checks by default. [CastOptions](https://github.com/apache/arrow/blob/446167169a0dc547e00e1bc29f417a40c0ca267a/python/pyarrow/_compute.pyx#L780-L795) also disables decimal precision truncation by default. The [Decimal32 definition](https://github.com/apache/arrow/blob/446167169a0dc547e00e1bc29f417a40c0ca267a/python/pyarrow/types.pxi#L4546-L4564) permits negative scales.
   
   | Input type and value | Destination | Expected | Observed |
   | --- | --- | --- | --- |
   | `decimal128(12,0)`, `5000000000` | `decimal32(9,0)` | Reject the ten-digit coefficient | `705032704` |
   | `decimal128(12,0)`, `536870912000` | `decimal32(9,-3)` | Preserve `536870912000` | `0E+3` |
   | `decimal128(12,2)`, `30000000.00` | `decimal32(9,0)` | Preserve `30000000` | `ArrowInvalid: Rescaling Decimal value would cause data loss` |
   
   The second destination coefficient is `536870912`. It fits nine digits exactly.
   
   #### Source
   
   [DecimalConversions<Decimal32, InDecimal>](https://github.com/apache/arrow/blob/446167169a0dc547e00e1bc29f417a40c0ca267a/cpp/src/arrow/compute/kernels/scalar_cast_numeric.cc#L487-L490) constructs Decimal32 from the low bits. [SafeRescaleDecimal](https://github.com/apache/arrow/blob/446167169a0dc547e00e1bc29f417a40c0ca267a/cpp/src/arrow/compute/kernels/scalar_cast_numeric.cc#L573-L589) then rescales and checks precision. Source inspection indicates that this order checks an already narrowed coefficient. Keeping the wider coefficient through rescaling and validation would address that ordering.
   
   #### Scope
   
   Verification covered Decimal64/128/256 sources, default and explicit safe casts, scalar, array, slice, chunked and null inputs. Wider rescaling, precision-edge and fractional-loss rejection controls passed. The inline example selects three default safe array cases. Other releases, platforms and source builds were not tested.
   
   The inline example is provided under CC0.
   
   Assistance covered the example and report text as well as discovery and verification.
   
   Found by ChipLab's AI verification agents and independently reproduced.
   
   ### Component(s)
   
   C++, Python
   
   ChipLab ref: CLAB-2026-257
   


-- 
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: issues-unsubscribe@arrow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] [C++][Python] Safe decimal narrowing to Decimal32 drops coefficient bits before checking [arrow]

Posted by "createcentury (via GitHub)" <gi...@apache.org>.
createcentury commented on issue #53288:
URL: https://github.com/apache/arrow/issues/53288#issuecomment-6129107576

   I had a look at this and the root cause is an ordering inversion in `DecimalConversions<Decimal32, InDecimal>`.
   
   The helper in `scalar_cast_numeric.cc` follows a convention its own comments spell out: widening conversions "convert then scale", narrowing ones "scale then truncate". `SafeRescaleDecimal` depends on it — it calls `ConvertInput` → `Rescale` → `FitsInPrecision` → `ConvertOutput`, so on every other narrowing path the overflow check sees the full-width coefficient. `DecimalConversions<Decimal64, Decimal128>` is the reference shape.
   
   The Decimal32 specialization is the only one that narrows in `ConvertInput`:
   
   ```cpp
   static Decimal32 ConvertInput(InDecimal&& val) { return Decimal32(val.low_bits()); }
   ```
   
   so both `Rescale` and `FitsInPrecision` run against an already-truncated coefficient — and this is the single implementation behind every `Decimal{32,64,128,256} -> Decimal32` cast.
   
   It accounts for the three reported cases exactly:
   
   - `5000000000 - 2^32 = 705032704`
   - `536870912000 = 125 * 2^32`, so the low 32 bits are `0`, which then rescales to `0E+3`
   - `3000000000 - 2^32 = -1294967296`, which is no longer divisible by 100, hence the spurious "would cause data loss"
   
   The fix is to give the specialization the same shape as the Decimal64 one: keep `InDecimal` through `ConvertInput` and narrow in `ConvertOutput`. `low_bits()` is defined on `SmallBasicDecimal` as well as on `BasicDecimal128`/`BasicDecimal256`, so a single generic specialization still covers all four source types.
   
   I'm putting together a PR with regression tests for the three cases. @vera-rubin, let me know if you were already working on one.
   


-- 
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@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org