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 <myzh...@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 <myzh...@google.com> Co-authored-by: Micah Kornfield <mic...@google.com> Signed-off-by: Micah Kornfield <emkornfi...@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) {