Copilot commented on code in PR #51169:
URL: https://github.com/apache/arrow/pull/51169#discussion_r4076240046
##########
cpp/src/arrow/util/decimal.cc:
##########
@@ -892,16 +904,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);
Review Comment:
This range check rejects the complete unscaled coefficient before callers
can apply a target-aware scale conversion. In particular, Gandiva's
`castDECIMAL_utf8` calls `Decimal128::FromString` through
`gdv_fn_dec_from_string` and only then calls `decimalops::Convert`; inputs such
as `1.55555555555555555555555555555555555555555555555555` for `DECIMAL(38,37)`
now fail and produce the cast error/zero instead of the documented HALF_UP
result. Please add the caller-side rounding/normalization (or another
target-aware parsing path) before `FromString`, with an end-to-end regression
test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
