dylanpulver opened a new issue, #24850:
URL: https://github.com/apache/datafusion/issues/24850

   ### Describe the bug
   
   Coercing two `Decimal256` types panics with `attempt to add with overflow` 
in debug builds. In release builds the `i8` wraps and the coercion silently 
returns the wrong precision.
   
   `get_wider_decimal_type` 
(`datafusion/expr-common/src/type_coercion/binary.rs:1213-1218`) and 
`get_wider_decimal_type_cross_variant` (`:1126-1128`) compute `max(s1, s2) + 
max(p1 - s1, p2 - s2)` entirely in `i8`, the type of a decimal *scale*:
   
   ```rust
   let s = *s1.max(s2);
   let range = (*p1 as i8 - s1).max(*p2 as i8 - s2);
   Some(create_decimal256_type((range + s) as u8, s))
   ```
   
   `DECIMAL256_MAX_PRECISION` and `DECIMAL256_MAX_SCALE` are both 76, so `p - 
s` reaches 152 and the sum reaches 228. `i8::MAX` is 127.
   
   `make_decimal_type` (`datafusion/sql/src/utils.rs:329`) accepts any `0 < 
precision <= 76` with `|scale| <= precision` and maps `precision > 38` to 
`Decimal256`, so `DECIMAL(76, 52)` is an ordinary user-written type.
   
   ### To Reproduce
   
   ```sql
   SELECT CAST(1 AS DECIMAL(76,0)) < CAST(2 AS DECIMAL(76,52));
   ```
   
   Panics during planning:
   
   ```
   thread 'main' panicked at 
datafusion/expr-common/src/type_coercion/binary.rs:1217:41:
   attempt to add with overflow
   ```
   
   Required precision here is `max(0, 52) + max(76 - 0, 76 - 52)` = 128.
   
   Also reachable without SQL: `comparison_coercion(&Decimal256(76, 0), 
&Decimal256(76, 52))`.
   
   ### Expected behavior
   
   The intermediate arithmetic should be done in a type that holds it, and the 
result clamped or rejected as `create_decimal*_type` already does.
   
   ### Additional context
   
   Fixing this alone does not make the query above execute. It moves the panic 
upstream to `arrow-cast/src/cast/decimal.rs:190`, which computes 
`(input_precision as i8) + delta_scale <= (output_precision as i8)` — the same 
class of bug. I measured both: with the coercion fixed, `create_logical_plan` 
succeeds and `collect()` panics in arrow-cast instead. The two overflows are 
the same quantity, so no SQL query can trigger one without the other; the arrow 
side needs a separate fix.
   
   The existing coercion tests top out around `Decimal256(30, 8)`, so the 
overflow region is not covered.
   
   Investigated with AI assistance (Claude Opus 4.8); the repro and the 
arrow-cast follow-on were run against `07483c1`, arrow 59.2.0, rustc 1.97.0.
   


-- 
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]

Reply via email to