danepitkin commented on code in PR #361:
URL: https://github.com/apache/arrow-nanoarrow/pull/361#discussion_r1455727949
##########
src/nanoarrow/utils.c:
##########
@@ -223,3 +223,186 @@ struct ArrowBufferAllocator ArrowBufferDeallocator(
allocator.private_data = private_data;
return allocator;
}
+
+static const int kInt32DecimalDigits = 9;
+
+static const uint64_t kUInt32PowersOfTen[] = {
+ 1ULL, 10ULL, 100ULL, 1000ULL, 10000ULL,
+ 100000ULL, 1000000ULL, 10000000ULL, 100000000ULL, 1000000000ULL};
+
+// Adapted from Arrow C++ to use 32-bit words for better C portability
+//
https://github.com/apache/arrow/blob/main/cpp/src/arrow/util/decimal.cc#L524-L544
+static void ShiftAndAdd(struct ArrowStringView value, uint32_t* out, int64_t
out_size) {
+ // We use strtoll for parsing, which needs input that is null-terminated
+ char chunk_string[16];
+
+ for (int64_t posn = 0; posn < value.size_bytes;) {
+ int64_t remaining = value.size_bytes - posn;
+
+ int64_t group_size;
+ if (remaining > kInt32DecimalDigits) {
+ group_size = kInt32DecimalDigits;
+ } else {
+ group_size = remaining;
+ }
+
+ const uint64_t multiple = kUInt32PowersOfTen[group_size];
+
+ memcpy(chunk_string, value.data + posn, group_size);
+ chunk_string[group_size] = '\0';
+ uint32_t chunk = (uint32_t)strtoll(chunk_string, NULL, 10);
+
+ for (int64_t i = 0; i < out_size; i++) {
+ uint64_t tmp = out[i];
+ tmp *= multiple;
+ tmp += chunk;
+ out[i] = (uint32_t)(tmp & 0xFFFFFFFFULL);
+ chunk = (uint32_t)(tmp >> 32);
+ }
+ posn += group_size;
+ }
+}
+
+ArrowErrorCode ArrowDecimalSetIntString(struct ArrowDecimal* decimal,
+ struct ArrowStringView value) {
+ // Check for sign
+ int is_negative = value.data[0] == '-';
+ int has_sign = is_negative || value.data[0] == '+';
+ value.data += has_sign;
+ value.size_bytes -= has_sign;
+
+ // Check all characters are digits that are not the negative sign
+ for (int64_t i = 0; i < value.size_bytes; i++) {
+ char c = value.data[i];
+ if (c < '0' || c > '9') {
+ return EINVAL;
+ }
Review Comment:
ignore me then! This code is already very readable IMO. Other parts of the
diff look like they are specifically written to avoid branching in loops (e.g.
`if` statements), so I wasn't sure if that was an active decision.
--
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]
