Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
1fanwang wants to merge 10 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
85821fd
GH-49817: [C++] Detect decimal parsing overflow
1fanwang Sep 6, 2026
ceed9bf
GH-49817: [C++] Handle signed decimal overflow
1fanwang Sep 7, 2026
c3bd939
GH-49817: [C++] Allow minimum Decimal32 and Decimal64
1fanwang Sep 7, 2026
7210c08
GH-49817: [C++] Validate metadata-only decimal parsing
1fanwang Sep 7, 2026
332b379
GH-49817: [C++] Reject overflowing decimal exponents
1fanwang Sep 22, 2026
9365c49
GH-49817: [C++] Fix decimal boundary tests on MSVC
1fanwang Sep 24, 2026
f3b69f9
GH-49817: [Gandiva] Round long strings before decimal conversion
1fanwang Sep 24, 2026
f496abd
GH-49817: [Gandiva] Preserve decimal exponent errors
1fanwang Sep 24, 2026
3eae100
GH-49817: [Gandiva] Reject oversized normalized decimals
1fanwang Sep 24, 2026
619808f
GH-49817: [Gandiva] Limit retries to checked scale reductions
1fanwang Sep 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions cpp/src/arrow/json/converter_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,15 @@ TEST(ConverterTest, Decimal128And256PrecisionError) {
std::shared_ptr<StructArray> parse_array;
ASSERT_OK(ParseFromString(options, json_source, &parse_array));

std::string error_msg =
"Invalid: Failed to convert JSON to " + decimal_type->ToString() +
": 123456789012345678901234567890.0123456789 requires precision 40";
std::string error_msg;
if (decimal_type->id() == Type::DECIMAL128) {
error_msg =
"Invalid: The string '123456789012345678901234567890.0123456789' "
"cannot be represented as decimal128";
} else {
error_msg = "Invalid: Failed to convert JSON to " + decimal_type->ToString() +
": 123456789012345678901234567890.0123456789 requires precision 40";
}
EXPECT_RAISES_WITH_MESSAGE_THAT(
Invalid, ::testing::HasSubstr(error_msg),
Convert(decimal_type, parse_array->GetFieldByName("")));
Expand Down
103 changes: 62 additions & 41 deletions cpp/src/arrow/util/decimal.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -768,7 +768,9 @@ std::string Decimal128::ToString(int32_t scale) const {
// Iterates over input and for each group of kInt64DecimalDigits multiple out by
// the appropriate power of 10 necessary to add source parsed as uint64 and
// then adds the parsed value of source.
static inline void ShiftAndAdd(std::string_view input, uint64_t out[], size_t out_size) {
static inline bool ShiftAndAddWithOverflow(std::string_view input, uint64_t out[],
size_t out_size, bool negative) {
constexpr uint64_t kSignBit = uint64_t{1} << 63;
for (size_t posn = 0; posn < input.size();) {
const size_t group_size = std::min(kInt64DecimalDigits, input.size() - posn);
const uint64_t multiple = kUInt64PowersOfTen[group_size];
Expand All @@ -783,20 +785,22 @@ static inline void ShiftAndAdd(std::string_view input, uint64_t out[], size_t ou
out[i] = static_cast<uint64_t>(tmp & 0xFFFFFFFFFFFFFFFFULL);
chunk = static_cast<uint64_t>(tmp >> 64);
}
if (chunk != 0) {
Comment thread
1fanwang marked this conversation as resolved.
return true;
}
const uint64_t high = out[out_size - 1];
if ((high & kSignBit) != 0 &&
(!negative || high != kSignBit ||
std::any_of(out, out + out_size - 1, [](uint64_t word) { return word != 0; }))) {
return true;
}
posn += group_size;
}
return false;
}

namespace {

struct DecimalComponents {
std::string_view whole_digits;
std::string_view fractional_digits;
int32_t exponent = 0;
char sign = 0;
bool has_exponent = false;
};

inline bool IsSign(char c) { return c == '-' || c == '+'; }

inline bool IsDot(char c) { return c == '.'; }
Expand All @@ -817,7 +821,10 @@ inline size_t ParseDigitsRun(const char* s, size_t start, size_t size,
return pos;
}

bool ParseDecimalComponents(const char* s, size_t size, DecimalComponents* out) {
} // namespace

bool internal::ParseDecimalComponents(const char* s, size_t size,
DecimalComponents* out) {
size_t pos = 0;

if (size == 0) {
Expand Down Expand Up @@ -859,15 +866,17 @@ bool ParseDecimalComponents(const char* s, size_t size, DecimalComponents* out)
return pos == size;
}

namespace {

template <typename Decimal>
Status DecimalFromString(const char* type_name, std::string_view s, Decimal* out,
int32_t* precision, int32_t* scale) {
if (s.empty()) {
return Status::Invalid("Empty string cannot be converted to ", type_name);
}

DecimalComponents dec;
if (!ParseDecimalComponents(s.data(), s.size(), &dec)) {
internal::DecimalComponents dec;
if (!internal::ParseDecimalComponents(s.data(), s.size(), &dec)) {
return Status::Invalid("The string '", s, "' is not a valid ", type_name, " number");
}

Expand All @@ -892,16 +901,17 @@ Status DecimalFromString(const char* type_name, std::string_view s, Decimal* out
parsed_scale = static_cast<int32_t>(dec.fractional_digits.size());
}

if (out != nullptr) {
static_assert(Decimal::kBitWidth % 64 == 0, "decimal bit-width not a multiple of 64");
std::array<uint64_t, Decimal::kBitWidth / 64> little_endian_array{};
ShiftAndAdd(dec.whole_digits, little_endian_array.data(), little_endian_array.size());
ShiftAndAdd(dec.fractional_digits, little_endian_array.data(),
little_endian_array.size());
*out = Decimal(bit_util::little_endian::ToNative(little_endian_array));
if (dec.sign == '-') {
out->Negate();
}
static_assert(Decimal::kBitWidth % 64 == 0, "decimal bit-width not a multiple of 64");
std::array<uint64_t, Decimal::kBitWidth / 64> little_endian_array{};
if (ShiftAndAddWithOverflow(dec.whole_digits, little_endian_array.data(),
little_endian_array.size(), dec.sign == '-') ||
ShiftAndAddWithOverflow(dec.fractional_digits, little_endian_array.data(),
little_endian_array.size(), dec.sign == '-')) {
return Status::Invalid("The string '", s, "' cannot be represented as ", type_name);
Comment thread
Copilot marked this conversation as resolved.
Comment thread
1fanwang marked this conversation as resolved.
}
Decimal parsed_value(bit_util::little_endian::ToNative(little_endian_array));
if (dec.sign == '-') {
parsed_value.Negate();
}

if (parsed_scale < 0) {
Expand All @@ -910,13 +920,19 @@ Status DecimalFromString(const char* type_name, std::string_view s, Decimal* out
if (-parsed_scale > Decimal::kMaxScale) {
return Status::Invalid("The string '", s, "' cannot be represented as ", type_name);
}
if (out != nullptr) {
*out *= Decimal::GetScaleMultiplier(-parsed_scale);
const auto& multiplier = Decimal::GetScaleMultiplier(-parsed_scale);
if (parsed_value > Decimal::GetMaxSentinel() / multiplier ||
parsed_value < Decimal::GetMinSentinel() / multiplier) {
return Status::Invalid("The string '", s, "' cannot be represented as ", type_name);
}
parsed_value *= multiplier;
parsed_precision -= parsed_scale;
parsed_scale = 0;
}

if (out != nullptr) {
*out = parsed_value;
}
if (precision != nullptr) {
*precision = parsed_precision;
}
Expand All @@ -934,8 +950,8 @@ Status SimpleDecimalFromString(const char* type_name, std::string_view s,
return Status::Invalid("Empty string cannot be converted to ", type_name);
}

DecimalComponents dec;
if (!ParseDecimalComponents(s.data(), s.size(), &dec)) {
internal::DecimalComponents dec;
if (!internal::ParseDecimalComponents(s.data(), s.size(), &dec)) {
return Status::Invalid("The string '", s, "' is not a valid ", type_name, " number");
}

Expand All @@ -960,19 +976,17 @@ Status SimpleDecimalFromString(const char* type_name, std::string_view s,
parsed_scale = static_cast<int32_t>(dec.fractional_digits.size());
}

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

*out = DecimalClass(value);
if (dec.sign == '-') {
out->Negate();
}
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);
}
DecimalClass parsed_value(value);
if (dec.sign == '-') {
parsed_value.Negate();
}

if (parsed_scale < 0) {
Expand All @@ -981,13 +995,20 @@ Status SimpleDecimalFromString(const char* type_name, std::string_view s,
if (-parsed_scale > DecimalClass::kMaxScale) {
return Status::Invalid("The string '", s, "' cannot be represented as ", type_name);
}
if (out != nullptr) {
*out *= DecimalClass::GetScaleMultiplier(-parsed_scale);
typename DecimalClass::ValueType scaled_value;
if (internal::MultiplyWithOverflow(
parsed_value.value(), DecimalClass::GetScaleMultiplier(-parsed_scale).value(),
&scaled_value)) {
return Status::Invalid("The string '", s, "' cannot be represented as ", type_name);
}
parsed_value = DecimalClass(scaled_value);
parsed_precision -= parsed_scale;
parsed_scale = 0;
}

if (out != nullptr) {
*out = parsed_value;
}
if (precision != nullptr) {
*precision = parsed_precision;
}
Expand Down
16 changes: 16 additions & 0 deletions cpp/src/arrow/util/decimal_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <array>
#include <cstdint>
#include <limits>
#include <string_view>
#include <type_traits>

#include "arrow/type_fwd.h"
Expand All @@ -30,6 +31,21 @@

namespace arrow {

namespace internal {

struct DecimalComponents {
std::string_view whole_digits;
std::string_view fractional_digits;
int32_t exponent = 0;
char sign = 0;
bool has_exponent = false;
};

ARROW_EXPORT bool ParseDecimalComponents(const char* s, size_t size,
DecimalComponents* out);

} // namespace internal

constexpr auto kInt32DecimalDigits =
static_cast<size_t>(std::numeric_limits<int32_t>::digits10);

Expand Down
Loading
Loading