more updates from comments

apache · Sep 30, 2024 · 1ae0185 · 1ae0185
1 parent 404921f
commit 1ae0185
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 60 deletions.
diff --git a/cpp/src/arrow/util/decimal.cc b/cpp/src/arrow/util/decimal.cc
@@ -753,23 +753,6 @@ std::string Decimal128::ToString(int32_t scale) const {
   return str;
 }
 
-static inline void ShiftAndAdd(std::string_view input, uint32_t* out) {
-  const uint32_t len =
-      static_cast<uint32_t>(std::min(kInt32DecimalDigits + 1, input.size()));
-  if (len == 0) {
-    return;
-  }
-
-  uint32_t value = 0;
-  ARROW_CHECK(internal::ParseValue<UInt32Type>(input.data(), len, &value));
-
-  uint64_t tmp = *out;
-  tmp *= kUInt32PowersOfTen[len];
-  tmp += value;
-
-  *out = static_cast<uint32_t>(tmp & 0xFFFFFFFFU);
-}
-
 // 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.
@@ -976,34 +959,17 @@ Status SimpleDecimalFromString(const char* type_name, std::string_view s,
   }
 
   if (out != nullptr) {
-    if constexpr (std::is_same_v<Decimal32, DecimalClass>) {
-      uint32_t value{0};
-      ShiftAndAdd(dec.whole_digits, &value);
-      ShiftAndAdd(dec.fractional_digits, &value);
-      if (value > static_cast<uint32_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();
-      }
-    } else {
-      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);
-      }
+    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();
-      }
+    *out = DecimalClass(value);
+    if (dec.sign == '-') {
+      out->Negate();
     }
   }
 

diff --git a/cpp/src/arrow/util/decimal_test.cc b/cpp/src/arrow/util/decimal_test.cc
@@ -1421,27 +1421,23 @@ TYPED_TEST(TestBasicDecimalFunctionality, TestFromBigEndianBadLength) {
 }
 
 TYPED_TEST(TestBasicDecimalFunctionality, TestToInteger) {
-  if constexpr (std::is_same_v<TypeParam, Decimal256>) {
-    GTEST_SKIP();  // Decimal256 doesn't have ToInteger
-  } else {
-    TypeParam value1("1234");
-    int32_t out1;
+  TypeParam value1("1234");
+  int32_t out1;
 
-    TypeParam value2("-1234");
-    int64_t out2;
+  TypeParam value2("-1234");
+  int64_t out2;
 
-    ASSERT_OK(value1.ToInteger(&out1));
-    ASSERT_EQ(1234, out1);
+  ASSERT_OK(value1.ToInteger(&out1));
+  ASSERT_EQ(1234, out1);
 
-    ASSERT_OK(value1.ToInteger(&out2));
-    ASSERT_EQ(1234, out2);
+  ASSERT_OK(value1.ToInteger(&out2));
+  ASSERT_EQ(1234, out2);
 
-    ASSERT_OK(value2.ToInteger(&out1));
-    ASSERT_EQ(-1234, out1);
+  ASSERT_OK(value2.ToInteger(&out1));
+  ASSERT_EQ(-1234, out1);
 
-    ASSERT_OK(value2.ToInteger(&out2));
-    ASSERT_EQ(-1234, out2);
-  }
+  ASSERT_OK(value2.ToInteger(&out2));
+  ASSERT_EQ(-1234, out2);
 }
 
 template <typename ArrowType, typename CType = typename ArrowType::c_type>