apache / apache/arrow-rs

Decimal arithmetic and decimal-to-decimal cast overflow `i8` precision/scale math for negative scales

Open
#10,978 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
3.6k
Forks
1.3k
Avg merge
2d 18h
Merged PRs (30d)
169

Description

### Describe the bug

Negative decimal scales are supported (see #10304), but two places compute precision/scale arithmetic in `i8` (the scale type) and overflow for values that are valid for the type. `Decimal256` allows a precision and scale of up to 76, so `p - s` reaches `76 - (-76) = 152` and `p + s` reaches `152`, both above `i8::MAX`. Debug builds panic; release builds wrap and produce a wrong precision.

1. `arrow-arith/src/numeric.rs`, `decimal_op` (add / sub / mul / div / rem result type, `main` line ~1050):

```rust
(result_scale.saturating_add((*p1 as i8 - s1).max(*p2 as i8 - s2)) as u8)
```

`*p1 as i8 - s1` is `76 - (-76)` for `Decimal256(76, -76)`.

2. `arrow-cast/src/cast/decimal.rs`, `is_infallible_cast` when casting to a larger scale (`main` line ~191):

```rust
let is_infallible_cast = (input_precision as i8) + delta_scale <= (output_precision as i8);
```

`delta_scale` is `output_scale - input_scale`, which is `0 - (-76) = 76`, and `76 + 76` overflows. (This one was also noted in apache/datafusion#24850.)

### To Reproduce

```rust
use arrow_array::{Decimal256Array, ArrayRef};
use arrow_buffer::i256;
use arrow_schema::DataType;
use std::sync::Arc;

let a: ArrayRef = Arc::new(Decimal256Array::from(vec![i256::from_i128(1)]).with_precision_and_scale(76, -76).unwrap());
let b: ArrayRef = Arc::new(Decimal256Array::from(vec![i256::from_i128(2)]).with_precision_and_scale(76, -76).unwrap());

// arrow-arith
let _ = arrow_arith::numeric::add(&a, &b);
// thread panicked at arrow-arith/src/numeric.rs:869:46 (59.2.0): attempt to subtract with overflow

// arrow-cast
let _ = arrow_cast::cast(&a, &DataType::Decimal256(76, 0));
// thread panicked at arrow-cast/src/cast/decimal.rs:190:30 (59.2.0): attempt to add with overflow
```

Through DataFusion (debug build of `datafusion-cli`):

```sql
SELECT arrow_cast(1, 'Decimal256(76,-76)') + arrow_cast(2, 'Decimal256(76,-76)');
SELECT arrow_cast(arrow_cast(1, 'Decimal256(76,-76)'), 'Decimal256(76,0)');
```

### Expected behavior

The intermediate arithmetic is done in a wider type (`i16`/`i32`) and the result clamped or rejected, so the kernels return a result or an `ArrowError` instead of panicking / wrapping.

### Additional context

Found while running a corpus of extreme-value literals through DataFusion. Line numbers above are from `arrow-cast` / `arrow-arith` 59.2.0; the same expressions are present on `main`.

Contributor guide

Open the contributing guide

Research direction

Start with the reproductions in the issue and inspect arrow-arith/src/numeric.rs at decimal_op and arrow-cast/src/cast/decimal.rs at is_infallible_cast. Trace the precision and scale calculations for Decimal256(76, -76), then add focused regression coverage for arithmetic and decimal-to-decimal casting. Done means valid inputs no longer panic or wrap and instead return the correct result or an ArrowError.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
data
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.