fix: avoid i8 overflow when computing the wider decimal precision

[dylanpulver]

Which issue does this PR close?

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

Rationale for this change

SELECT CAST(1 AS DECIMAL(76,0)) < CAST(2 AS DECIMAL(76,52)) panics during planning. The required precision max(s1, s2) + max(p1 - s1, p2 - s2) is computed in i8, the type of a decimal scale, but Decimal256 allows a precision and a scale of 76, so p - s reaches 152 and the sum reaches 228. Debug builds panic; release builds wrap and return a wrong precision.

What changes are included in this PR?

The expression moves into required_decimal_precision(p1, s1, p2, s2), which computes in i32 and clamps into u8. The five call sites in get_wider_decimal_type and get_wider_decimal_type_cross_variant use it.

The change is inert on inputs that did not overflow. With s = max(s1, s2) = s1 without loss of generality, range >= p1 - s1, so range + s >= p1 >= 0: the sum is never negative, the lower clamp never fires, and for values in 0..=127 the widened computation equals the old one bit for bit.

What is the testing strategy for this PR?

test_decimal256_comparison_coercion_precision_overflow in binary/tests/comparison.rs, three cases: sum 128, sum 152, and a negative scale where p1 - s1 is 152 before the sum is computed.

cargo test -p datafusion-expr-common --lib: 207 passed on main, 208 with this branch. Reverting the source with the test kept panics at binary.rs:1217:41 with attempt to add with overflow. The naive fix — range.saturating_add(s) at all five sites — still fails, now at binary.rs:1216:25 with attempt to subtract with overflow, which is why the negative-scale case is in the test.

End to end, same probe both ways: on main create_logical_plan for the query above panics at binary.rs:1217:41; with this branch it returns Projection: CAST(Int64(1) AS Decimal256(76, 0)) < CAST(Int64(2) AS Decimal256(76, 52)) AS r.

cargo fmt --all --check, cargo clippy -p datafusion-expr-common --all-targets --all-features -- -D warnings, and cargo test on datafusion-expr-common, datafusion-expr, datafusion-sql, datafusion-optimizer all pass. rustc 1.97.0, matching rust-toolchain.toml.

No .slt case, deliberately — see below.

Are there any user-facing changes?

Two things a reviewer should weigh, both stated because I am not certain they are what you want:

This does not make the query execute. It moves the panic to arrow-cast/src/cast/decimal.rs:190, (input_precision as i8) + delta_scale <= (output_precision as i8), which is the same bug upstream. I measured it: after this fix, planning succeeds and collect() panics there. The two overflows are the same quantity, so no SQL can trigger DataFusion's without also triggering arrow's. That is why there is no .slt case — one would fail CI. The arrow side needs its own fix.

One release-mode behaviour change. In get_wider_decimal_type_cross_variant with a negative scale, the wrapped i8 previously produced a small required_precision that passed the variant checks and yielded a lossy type; it now yields 152, exceeds every maximum, and returns None. Negative scales cannot be written in DDL, but they can arrive from directly constructed DataTypes such as Parquet or IPC schemas. I believe None is correct — there is no common type that holds both — but it is a change, not a no-op, and it is the one part of this I would want a second opinion on.

Not run: the workspace-wide clippy and the extended-features test run from AGENTS.md, ./dev/rust_lint.sh as a whole, a release build, and benchmarks. The sqllogictest suite reports the same 39 failures with and without this change, all from an uninitialised parquet-testing submodule in my checkout.

---

Per the AI-assisted contributions policy: this patch, its test and the measurements above were produced with AI assistance (Claude Opus 5). The unknowns are the two items in the section above.