[arrow] branch master updated: ARROW-9671: [C++] Fix a bug in BasicDecimal128 constructor that interprets uint64_t integers with highest bit set as negative.

[em...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

emkornfield pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/master by this push:
     new 3d0a9d5  ARROW-9671: [C++] Fix a bug in BasicDecimal128 constructor that interprets uint64_t integers with highest bit set as negative.
3d0a9d5 is described below

commit 3d0a9d58b6fe29dcb208c3fa244c789449517988
Author: Mingyu Zhong <my...@google.com>
AuthorDate: Fri Aug 7 19:56:01 2020 -0700

    ARROW-9671: [C++] Fix a bug in BasicDecimal128 constructor that interprets uint64_t integers with highest bit set as negative.
    
    Closes #7915 from MingyuZhong/bn
    
    Lead-authored-by: Mingyu Zhong <my...@google.com>
    Co-authored-by: Micah Kornfield <mi...@google.com>
    Signed-off-by: Micah Kornfield <em...@gmail.com>
---
 cpp/src/arrow/util/basic_decimal.h |  7 ++++---
 cpp/src/arrow/util/decimal_test.cc | 22 ++++++++++++++++------
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/cpp/src/arrow/util/basic_decimal.h b/cpp/src/arrow/util/basic_decimal.h
index 01feeac..23c38bb 100644
--- a/cpp/src/arrow/util/basic_decimal.h
+++ b/cpp/src/arrow/util/basic_decimal.h
@@ -51,10 +51,11 @@ class ARROW_EXPORT BasicDecimal128 {
 
   /// \brief Convert any integer value into a BasicDecimal128.
   template <typename T,
-            typename = typename std::enable_if<std::is_integral<T>::value, T>::type>
+            typename = typename std::enable_if<
+                std::is_integral<T>::value && (sizeof(T) <= sizeof(uint64_t)), T>::type>
   constexpr BasicDecimal128(T value) noexcept
-      : BasicDecimal128(static_cast<int64_t>(value) >= 0 ? 0 : -1,
-                        static_cast<uint64_t>(value)) {}
+      : BasicDecimal128(value >= T{0} ? 0 : -1, static_cast<uint64_t>(value)) {  // NOLINT
+  }
 
   /// \brief Create a BasicDecimal128 from an array of bytes. Bytes are assumed to be in
   /// native-endian byte order.
diff --git a/cpp/src/arrow/util/decimal_test.cc b/cpp/src/arrow/util/decimal_test.cc
index b62992c..856f10e 100644
--- a/cpp/src/arrow/util/decimal_test.cc
+++ b/cpp/src/arrow/util/decimal_test.cc
@@ -218,8 +218,7 @@ TEST(DecimalZerosTest, NoLeadingZerosDecimalPoint) {
 template <typename T>
 class Decimal128Test : public ::testing::Test {
  public:
-  Decimal128Test() : value_(42) {}
-  const T value_;
+  Decimal128Test() {}
 };
 
 using Decimal128Types =
@@ -231,18 +230,29 @@ using Decimal128Types =
 TYPED_TEST_SUITE(Decimal128Test, Decimal128Types);
 
 TYPED_TEST(Decimal128Test, ConstructibleFromAnyIntegerType) {
-  Decimal128 value(this->value_);
-  ASSERT_EQ(42, value.low_bits());
+  Decimal128 value(TypeParam{42});
+  EXPECT_EQ(42, value.low_bits());
+  EXPECT_EQ(0, value.high_bits());
+
+  Decimal128 max_value(std::numeric_limits<TypeParam>::max());
+  EXPECT_EQ(std::numeric_limits<TypeParam>::max(), max_value.low_bits());
+  EXPECT_EQ(0, max_value.high_bits());
+
+  Decimal128 min_value(std::numeric_limits<TypeParam>::min());
+  EXPECT_EQ(std::numeric_limits<TypeParam>::min(), min_value.low_bits());
+  EXPECT_EQ((std::is_signed<TypeParam>::value ? -1 : 0), min_value.high_bits());
 }
 
 TEST(Decimal128TestTrue, ConstructibleFromBool) {
   Decimal128 value(true);
-  ASSERT_EQ(1, value.low_bits());
+  EXPECT_EQ(1, value.low_bits());
+  EXPECT_EQ(0, value.high_bits());
 }
 
 TEST(Decimal128TestFalse, ConstructibleFromBool) {
   Decimal128 value(false);
-  ASSERT_EQ(0, value.low_bits());
+  EXPECT_EQ(0, value.low_bits());
+  EXPECT_EQ(0, value.high_bits());
 }
 
 TEST(Decimal128Test, Division) {