pitrou commented on code in PR #51169:
URL: https://github.com/apache/arrow/pull/51169#discussion_r3948274157
##########
cpp/src/arrow/util/decimal.cc:
##########
@@ -783,8 +784,25 @@ 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) {
Review Comment:
Should we also check `out[size - 1] & kSignBit` to detect overflow into the
sign bit here?
##########
cpp/src/arrow/util/decimal_test.cc:
##########
@@ -550,6 +555,9 @@ TEST(Decimal256Test, FromStringLimits) {
//
Decimal256::FromString("-9999999999999999999999999999999999999999999999999999999999999999999999999999e1"));
// ASSERT_RAISES(Invalid,
//
Decimal256::FromString("99999999999999999999999999999999999999999999999999999999999999999999999999999"));
+ ASSERT_RAISES(Invalid, Decimal256::FromString(std::string(78, '9')));
+ ASSERT_RAISES(Invalid,
Decimal256::FromString("5789604461865809771178549250434395392663"
+
"4992332820282019728792003956564819968"));
Review Comment:
Also check negative overflow?
##########
cpp/src/arrow/util/decimal.cc:
##########
@@ -783,8 +784,25 @@ 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) {
+ return true;
+ }
posn += group_size;
}
+ return false;
+}
+
+static inline bool MagnitudeOverflowsSignedDecimal(const uint64_t out[],
size_t out_size,
Review Comment:
I'm not sure we need a separate function since the check can probably be
done in `ShiftAndAddWithOverflow` (see previous comment above).
--
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]
