Skip to content

Avoid overflow in decimal precision and scale arithmetic - #10996

Open
snoopuppy582 wants to merge 1 commit into
apache:mainfrom
snoopuppy582:fix/10978-decimal-metadata-overflow
Open

Avoid overflow in decimal precision and scale arithmetic#10996
snoopuppy582 wants to merge 1 commit into
apache:mainfrom
snoopuppy582:fix/10978-decimal-metadata-overflow

Conversation

@snoopuppy582

@snoopuppy582 snoopuppy582 commented Sep 5, 2026

Copy link
Copy Markdown

Which issue does this PR close?

Closes #10978.

Rationale for this change

Valid negative decimal scales can overflow i8 precision/scale calculations, causing debug panics or incorrect metadata and cast fast-path selection when overflow checks are disabled.

What changes are included in this PR?

  • Compute add/subtract/remainder precision and scale differences in i16, clamping precision before narrowing.
  • Reject unrepresentable multiplication scales and overflowing remainder alignment factors.
  • Widen decimal cast scale differences and precision checks while preserving existing rounding, nulls, and safe/unsafe cast behavior.

Are these changes tested?

The six new regression tests fail on current main before the fix.

  • cargo +stable test -p arrow-arith -p arrow-cast --lib: 623 passed.
  • cargo +stable --config 'profile.test.package.arrow-arith.overflow-checks=false' --config 'profile.test.package.arrow-cast.overflow-checks=false' test -p arrow-arith -p arrow-cast --lib decimal256 --verbose: 31 passed. This is an unchecked test profile, not an optimized release build.
  • cargo +stable fmt --all -- --check and git diff --check: passed.
  • cargo +stable clippy -p arrow-arith -p arrow-cast --lib --tests --no-deps -- -D warnings -A clippy::chunks_exact_to_as_chunks: passed.

Local checks used Rust 1.98.0 on Windows because the installed repository-pinned 1.97.1 toolchain was incomplete. Strict Clippy first reported the new chunks_exact_to_as_chunks lint in unchanged arrow-buffer and arrow-arith/src/aggregate.rs code. The scoped run allows only that lint; the repository's existing removed-lint configuration also emits a compatibility warning. No source or lint configuration was changed for these diagnostics.

Are there any user-facing changes?

Affected addition, subtraction, multiplication, remainder, and decimal casts now return correctly calculated results or checked errors for extreme negative scales instead of panicking or wrapping metadata. Public API signatures are unchanged.

AI assistance: OpenAI Codex (GPT-6) generated the implementation, regression tests, and this description. Codex agents reviewed the diff and ran the checks listed above. I have personally reviewed the code in this PR.

@github-actions github-actions Bot added arrow Changes to the arrow crate arrow-cast arrow-arith labels Sep 5, 2026
@Jefffrey Jefffrey added the bug label Sep 6, 2026

@ryux1 ryux1 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked the widened precision/scale formulas at i8::MIN, the Decimal256 +/-76 limits, and cross-scale differences up to 204. The add/sub precision expression remains positive because the operand at max(scale) contributes its own precision; the remainder expression is likewise at least one, while both clamp before narrowing. Multiply now rejects a sum below i8::MIN explicitly, and cast fast-path selection no longer wraps in debug or unchecked profiles. Checked powers replace the erroneous remainder wrapping path. The new tests exercise safe/unsafe casts and overflow-check-disabled arithmetic, and the full reported CI matrix is green.

@Jefffrey

Jefffrey commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

@ryux1 are you an LLM agent? i cant help notice youve been mass commenting across discussions and pull requests and these messages read very much like a LLM

@ryux1

ryux1 commented Sep 6, 2026

Copy link
Copy Markdown

Yes — I have been using an AI coding assistant for some of these reviews. You are right that the recent volume and wording came across as automated. I am going to stop the drive-by reviews and only comment when I have a concrete, independently checked issue to add. Sorry for the noise.

@Jefffrey

Jefffrey commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

@ryux1 this reads as an LLM apology... please refrain from spamming LLM comments unless you feel it has something to add, and when you do please clearly indicate it is from an LLM

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

3 participants