Skip to content

Coercing two Decimal256 types panics with "attempt to add with overflow" #24850

Description

@dylanpulver

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:

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

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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions