pitrou commented on a change in pull request #8542:
URL: https://github.com/apache/arrow/pull/8542#discussion_r524305460
##########
File path: cpp/src/arrow/util/basic_decimal.cc
##########
@@ -490,49 +529,64 @@ static void FixDivisionSigns(BasicDecimal128* result,
BasicDecimal128* remainder
}
}
-/// \brief Build a BasicDecimal128 from a list of ints.
-static DecimalStatus BuildFromArray(BasicDecimal128* value, uint32_t* array,
- int64_t length) {
- switch (length) {
- case 0:
- *value = {static_cast<int64_t>(0)};
- break;
- case 1:
- *value = {static_cast<int64_t>(array[0])};
- break;
- case 2:
- *value = {static_cast<int64_t>(0),
- (static_cast<uint64_t>(array[0]) << 32) + array[1]};
- break;
- case 3:
- *value = {static_cast<int64_t>(array[0]),
- (static_cast<uint64_t>(array[1]) << 32) + array[2]};
- break;
- case 4:
- *value = {(static_cast<int64_t>(array[0]) << 32) + array[1],
- (static_cast<uint64_t>(array[2]) << 32) + array[3]};
- break;
- case 5:
- if (array[0] != 0) {
- return DecimalStatus::kOverflow;
- }
- *value = {(static_cast<int64_t>(array[1]) << 32) + array[2],
- (static_cast<uint64_t>(array[3]) << 32) + array[4]};
- break;
- default:
+/// \brief Build a little endian array of uint64_t from a big endian array of
uint32_t.
+template <size_t N>
+static DecimalStatus BuildFromArray(std::array<uint64_t, N>* result_array,
+ const uint32_t* array, int64_t length) {
+ for (int64_t i = length - 2 * N - 1; i >= 0; i--) {
+ if (array[i] != 0) {
return DecimalStatus::kOverflow;
+ }
+ }
+ int64_t next_index = length - 1;
+ size_t i = 0;
+ for (; i < N && next_index >= 0; i++) {
+ uint64_t lower_bits = array[next_index--];
+ (*result_array)[i] =
+ (next_index < 0)
+ ? lower_bits
+ : ((static_cast<uint64_t>(array[next_index--]) << 32) +
lower_bits);
+ }
+ for (; i < N; i++) {
+ (*result_array)[i] = 0;
}
+ return DecimalStatus::kSuccess;
+}
+/// \brief Build a BasicDecimal128 from a big endian array of uint32_t.
+static DecimalStatus BuildFromArray(BasicDecimal128* value, const uint32_t*
array,
+ int64_t length) {
+ std::array<uint64_t, 2> result_array;
+ auto status = BuildFromArray(&result_array, array, length);
+ if (status != DecimalStatus::kSuccess) {
+ return status;
+ }
+ *value = {static_cast<int64_t>(result_array[1]), result_array[0]};
+ return DecimalStatus::kSuccess;
+}
+
+/// \brief Build a BasicDecimal256 from a big endian array of uint32_t.
+static DecimalStatus BuildFromArray(BasicDecimal256* value, const uint32_t*
array,
+ int64_t length) {
+ std::array<uint64_t, 4> result_array;
+ auto status = BuildFromArray(&result_array, array, length);
+ if (status != DecimalStatus::kSuccess) {
+ return status;
+ }
+ *value = result_array;
return DecimalStatus::kSuccess;
}
/// \brief Do a division where the divisor fits into a single 32 bit value.
-static DecimalStatus SingleDivide(const uint32_t* dividend, int64_t
dividend_length,
- uint32_t divisor, BasicDecimal128* remainder,
- bool dividend_was_negative, bool
divisor_was_negative,
- BasicDecimal128* result) {
+template <class DecimalClass>
+static inline DecimalStatus SingleDivide(const uint32_t* dividend,
+ int64_t dividend_length, uint32_t
divisor,
+ DecimalClass* remainder,
+ bool dividend_was_negative,
+ bool divisor_was_negative,
+ DecimalClass* result) {
uint64_t r = 0;
- uint32_t result_array[5];
+ uint32_t* result_array = new uint32_t[dividend_length];
Review comment:
Is the dynamic allocation really necessary? Isn't a local array of
length 9 sufficient?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
