GH-49817: [C++] Reject overflowing decimal strings

[1fanwang]

Rationale for this change

Parsing decimals can wrap oversized inputs. Gandiva must still round strings that fit the requested type.

Closes #49817.

What changes are included in this PR?

Check signed overflow, including metadata-only calls. Gandiva retries only when checked scale arithmetic permits reduction and the scaled coefficient fits 38 digits.

Are these changes tested?

I tested on macOS arm64 with Apple clang 21 and LLVM 19.1.7.

Raw logs

cmake -S cpp -B build -DARROW_DEPENDENCY_SOURCE=BUNDLED -DARROW_BUILD_TESTS=ON -DARROW_GANDIVA=ON -DLLVM_ROOT=/opt/homebrew/opt/llvm@19
cmake --build build --target gandiva-projector-test -j6
mkdir -p .scratch

clang++ -std=c++20 -Icpp/src -Ibuild/src -Lbuild/release -Wl,-rpath,"$PWD/build/release" -larrow -x c++ -o .scratch/decimal-probe - <<'CPP'
#include <iostream>
#include "arrow/util/decimal.h"
int main() {
  arrow::Decimal128 value;
  auto status = arrow::Decimal128::FromString(
      "1." + std::string(50, '5'), &value, nullptr, nullptr);
  std::cout << status << '\n';
  if (status.ok()) std::cout << value.ToIntegerString() << '\n';
}
CPP
.scratch/decimal-probe

Before:

OK
151473969238762845885664163847966963939

With this PR:

Invalid: The string '1.55555555555555555555555555555555555555555555555555' cannot be represented as decimal128

LLVM casts reject 50 integer digits followed by 40 fractional digits and 1e-2147483648. At scale 0, 1e-30 still returns zero.

build/release/gandiva-projector-test --gtest_filter='TestDecimal.*'

Before d4eeabe:

Value of: status.ok()
  Actual: true
Expected: false

After:

[       OK ] TestDecimal.TestCastDecimalVarCharInvalidInput (40 ms)
[  PASSED  ] 22 tests.

Are there any user-facing changes?

Parsing rejects overflow; Gandiva retains HALF_UP rounding for long strings.

This PR contains a "Critical Fix".

Was AI used for this PR?

In accordance to the AI generation guidelines, please disclose below whether and how AI was used in this PR.

PR code and description written by:

• Human
• AI

Reviewed before submission by:

• Human
• AI
• Not reviewed

• GitHub Issue: [C++] arrow::Decimal128::FromString silently truncates when the input string has more than 38 significant digits #49817

[github-actions]

⚠️ GitHub issue #49817 has been automatically assigned in GitHub to PR creator.

[Copilot]

🟡 Changes recommended

Decimal32/Decimal64 parsing still incorrectly rejects the minimum negative representable value due to a value > max() magnitude check that doesn’t allow -2^(N-1).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR addresses integer overflow during C++ decimal string parsing so that oversized inputs no longer wrap modulo the destination bit width while still returning Status::OK(). It does so by adding overflow detection to the digit-accumulation path and rejecting magnitudes outside the signed integer range before constructing Decimal128/Decimal256 values.

Changes:

• Add carry/overflow reporting to the digit accumulator used by FromString so overflow is detected instead of silently dropped.
• Reject parsed magnitudes outside the signed representable range (e.g., > 2^(N-1)-1, or < -2^(N-1)) for wide decimals prior to constructing the decimal value.
• Extend Decimal128Test/Decimal256Test limit coverage with overflow-focused cases matching the reported issue.

File summaries

File	Description
cpp/src/arrow/util/decimal.cc	Introduces overflow-aware accumulation and signed-range magnitude rejection during decimal parsing.
cpp/src/arrow/util/decimal_test.cc	Adds regression tests ensuring oversized Decimal128/Decimal256 strings now return Invalid.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Decimal32/Decimal64 parsing still incorrectly rejects the smallest negative in-range value due to a sign-insensitive magnitude check in the updated overflow-aware path.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Overflow rejection is currently conditional on out != nullptr, so callers requesting only precision/scale can still receive OK for unrepresentable inputs.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

cpp/src/arrow/util/decimal.cc:986

• Same issue as above: representability/overflow checks are skipped entirely when out == nullptr, so callers asking only for precision/scale could get OK for values that wouldn't fit in the destination type.

  if (out != nullptr) {
    uint64_t value{0};
    if (ShiftAndAddWithOverflow(dec.whole_digits, &value, 1, dec.sign == '-') ||
        ShiftAndAddWithOverflow(dec.fractional_digits, &value, 1, dec.sign == '-') ||
        value > static_cast<uint64_t>(
                    std::numeric_limits<typename DecimalClass::ValueType>::max()) +
                    static_cast<uint64_t>(dec.sign == '-')) {
      return Status::Invalid("The string '", s, "' cannot be represented as ", type_name);
    }

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The overflow detection is implemented in the shared accumulator path, is applied consistently across decimal widths, and is backed by focused boundary/overflow tests that cover the reported failure mode.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

[github-actions]

⚠️ GitHub issue #49817 has been automatically assigned in GitHub to PR creator.

[Copilot]

Copilot review overview

🟡 Changes recommended

Positive-exponent scaling and metadata-only parsing can still bypass overflow validation.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Detect overflow when scaling decimals by positive exponents · New

[github-actions]

⚠️ GitHub issue #49817 has been automatically assigned in GitHub to PR creator.

[Copilot]

Copilot review overview

🟡 Changes recommended

Target-aware rounding is still needed before Gandiva conversion for values requiring HALF_UP normalization.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Apply target-aware rounding before Decimal128 range validation · New

Resolved since last review (1)

• Detect overflow when scaling decimals by positive exponents

[1fanwang]

Done in cccb72a.

[github-actions]

⚠️ GitHub issue #49817 has been automatically assigned in GitHub to PR creator.

[Copilot]

Copilot review overview

🟡 Changes recommended

Fix the three unresolved Gandiva fallback issues, including the critical conversion of parser errors into zero.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2

Open (2)

• Retry fallback masks Decimal128 exponent range errors · New
• Apply target-aware rounding before Decimal128 range validation

[github-actions]

⚠️ GitHub issue #49817 has been automatically assigned in GitHub to PR creator.

[github-actions]

⚠️ GitHub issue #49817 has been automatically assigned in GitHub to PR creator.

[Copilot]

Copilot review overview

🔵 Needs a closer look

The changes span core decimal parsing and Gandiva behavior, warranting final human review.

Review effort: Lite
Findings: 1

Open (1)

• Apply target-aware rounding before Decimal128 range validation

Resolved since last review (1)

• Retry fallback masks Decimal128 exponent range errors

[github-actions]

⚠️ GitHub issue #49817 has been automatically assigned in GitHub to PR creator.

[github-actions]

⚠️ GitHub issue #49817 has been automatically assigned in GitHub to PR creator.

[Copilot]

Copilot review overview

🟡 Changes recommended

Rounded Gandiva results can bypass the precision check and emit out-of-range values.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2

Open (2)

• Rounded overflow bypasses out_precision validation · New
• Apply target-aware rounding before Decimal128 range validation

[1fanwang]

Convert still checks precision at the same scale, and this input returns 0.00 through the projector.

[github-actions]

⚠️ GitHub issue #49817 has been automatically assigned in GitHub to PR creator.

[Copilot]

Copilot review overview

🟢 Approval recommended

No unresolved blocking issues were identified.

Review effort: Lite
Findings: 2

Open (2)

• Rounded overflow bypasses out_precision validation
• Apply target-aware rounding before Decimal128 range validation