Repository: arrow Updated Branches: refs/heads/master 3fbf76041 -> cf1ac9cf4
ARROW-1531: [C++] Return ToBytes by value from Decimal128 Author: Phillip Cloud <cpcl...@gmail.com> Closes #1094 from cpcloud/fix-decimal128-to-bytes and squashes the following commits: 622fd99f [Phillip Cloud] ARROW-1531: [C++] Use forward references for appending std::array in FixedSizeBinaryBuilder Project: http://git-wip-us.apache.org/repos/asf/arrow/repo Commit: http://git-wip-us.apache.org/repos/asf/arrow/commit/cf1ac9cf Tree: http://git-wip-us.apache.org/repos/asf/arrow/tree/cf1ac9cf Diff: http://git-wip-us.apache.org/repos/asf/arrow/diff/cf1ac9cf Branch: refs/heads/master Commit: cf1ac9cf4d82befadec3e40386daeff6aab0de90 Parents: 3fbf760 Author: Phillip Cloud <cpcl...@gmail.com> Authored: Tue Sep 12 17:24:19 2017 -0400 Committer: Wes McKinney <wes.mckin...@twosigma.com> Committed: Tue Sep 12 17:24:19 2017 -0400 ---------------------------------------------------------------------- cpp/src/arrow/array-test.cc | 4 +--- cpp/src/arrow/buffer.h | 2 +- cpp/src/arrow/builder.cc | 4 +--- cpp/src/arrow/builder.h | 1 + cpp/src/arrow/util/decimal-test.cc | 11 +++-------- cpp/src/arrow/util/decimal.cc | 11 ++++------- cpp/src/arrow/util/decimal.h | 10 ++++------ 7 files changed, 15 insertions(+), 28 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/arrow/blob/cf1ac9cf/cpp/src/arrow/array-test.cc ---------------------------------------------------------------------- diff --git a/cpp/src/arrow/array-test.cc b/cpp/src/arrow/array-test.cc index 39db715..9731083 100644 --- a/cpp/src/arrow/array-test.cc +++ b/cpp/src/arrow/array-test.cc @@ -2577,10 +2577,8 @@ class DecimalTest : public ::testing::TestWithParam<int> { void MakeData(const DecimalVector& input, std::vector<uint8_t>* out) const { out->reserve(input.size() * BYTE_WIDTH); - std::array<uint8_t, BYTE_WIDTH> bytes{{0}}; - for (const auto& value : input) { - ASSERT_OK(value.ToBytes(&bytes)); + auto bytes = value.ToBytes(); out->insert(out->end(), bytes.cbegin(), bytes.cend()); } } http://git-wip-us.apache.org/repos/asf/arrow/blob/cf1ac9cf/cpp/src/arrow/buffer.h ---------------------------------------------------------------------- diff --git a/cpp/src/arrow/buffer.h b/cpp/src/arrow/buffer.h index 4c3bd79..d215267 100644 --- a/cpp/src/arrow/buffer.h +++ b/cpp/src/arrow/buffer.h @@ -219,7 +219,7 @@ class ARROW_EXPORT BufferBuilder { template <size_t NBYTES> Status Append(const std::array<uint8_t, NBYTES>& data) { constexpr auto nbytes = static_cast<int64_t>(NBYTES); - if (capacity_ < static_cast<int64_t>(nbytes) + size_) { + if (capacity_ < nbytes + size_) { int64_t new_capacity = BitUtil::NextPower2(nbytes + size_); RETURN_NOT_OK(Resize(new_capacity)); } http://git-wip-us.apache.org/repos/asf/arrow/blob/cf1ac9cf/cpp/src/arrow/builder.cc ---------------------------------------------------------------------- diff --git a/cpp/src/arrow/builder.cc b/cpp/src/arrow/builder.cc index b27b2c7..a194ab7 100644 --- a/cpp/src/arrow/builder.cc +++ b/cpp/src/arrow/builder.cc @@ -1119,9 +1119,7 @@ DecimalBuilder::DecimalBuilder(MemoryPool* pool, const std::shared_ptr<DataType> Status DecimalBuilder::Append(const Decimal128& value) { RETURN_NOT_OK(FixedSizeBinaryBuilder::Reserve(1)); - std::array<uint8_t, 16> bytes; - RETURN_NOT_OK(value.ToBytes(&bytes)); - return FixedSizeBinaryBuilder::Append(bytes); + return FixedSizeBinaryBuilder::Append(value.ToBytes()); } Status DecimalBuilder::Finish(std::shared_ptr<Array>* out) { http://git-wip-us.apache.org/repos/asf/arrow/blob/cf1ac9cf/cpp/src/arrow/builder.h ---------------------------------------------------------------------- diff --git a/cpp/src/arrow/builder.h b/cpp/src/arrow/builder.h index 80c63a5..4d18a3e 100644 --- a/cpp/src/arrow/builder.h +++ b/cpp/src/arrow/builder.h @@ -18,6 +18,7 @@ #ifndef ARROW_BUILDER_H #define ARROW_BUILDER_H +#include <array> #include <cstdint> #include <functional> #include <limits> http://git-wip-us.apache.org/repos/asf/arrow/blob/cf1ac9cf/cpp/src/arrow/util/decimal-test.cc ---------------------------------------------------------------------- diff --git a/cpp/src/arrow/util/decimal-test.cc b/cpp/src/arrow/util/decimal-test.cc index 6f5118e..565a1bb 100644 --- a/cpp/src/arrow/util/decimal-test.cc +++ b/cpp/src/arrow/util/decimal-test.cc @@ -101,9 +101,7 @@ TEST(DecimalTest, TestFromDecimalString128) { TEST(DecimalTest, TestDecimal32SignedRoundTrip) { Decimal128 expected("-3402692"); - std::array<uint8_t, 16> bytes; - ASSERT_OK(expected.ToBytes(&bytes)); - + auto bytes = expected.ToBytes(); Decimal128 result(bytes.data()); ASSERT_EQ(expected, result); } @@ -113,9 +111,7 @@ TEST(DecimalTest, TestDecimal64SignedRoundTrip) { std::string string_value("-34034293045.921"); ASSERT_OK(Decimal128::FromString(string_value, &expected)); - std::array<uint8_t, 16> bytes; - ASSERT_OK(expected.ToBytes(&bytes)); - + auto bytes = expected.ToBytes(); Decimal128 result(bytes.data()); ASSERT_EQ(expected, result); @@ -131,8 +127,7 @@ TEST(DecimalTest, TestDecimalStringAndBytesRoundTrip) { ASSERT_EQ(expected, expected_underlying_value); - std::array<uint8_t, 16> bytes; - ASSERT_OK(expected.ToBytes(&bytes)); + auto bytes = expected.ToBytes(); Decimal128 result(bytes.data()); http://git-wip-us.apache.org/repos/asf/arrow/blob/cf1ac9cf/cpp/src/arrow/util/decimal.cc ---------------------------------------------------------------------- diff --git a/cpp/src/arrow/util/decimal.cc b/cpp/src/arrow/util/decimal.cc index 123482e..49d5c02 100644 --- a/cpp/src/arrow/util/decimal.cc +++ b/cpp/src/arrow/util/decimal.cc @@ -44,15 +44,12 @@ Decimal128::Decimal128(const uint8_t* bytes) : Decimal128(reinterpret_cast<const int64_t*>(bytes)[0], reinterpret_cast<const uint64_t*>(bytes)[1]) {} -Status Decimal128::ToBytes(std::array<uint8_t, 16>* out) const { - if (out == nullptr) { - return Status::Invalid("Cannot fill nullptr of bytes from Decimal128"); - } - +std::array<uint8_t, 16> Decimal128::ToBytes() const { const uint64_t raw[] = {static_cast<uint64_t>(high_bits_), low_bits_}; const auto* raw_data = reinterpret_cast<const uint8_t*>(raw); - std::copy(raw_data, raw_data + out->size(), out->begin()); - return Status::OK(); + std::array<uint8_t, 16> out{{0}}; + std::copy(raw_data, raw_data + out.size(), out.begin()); + return out; } std::string Decimal128::ToString(int precision, int scale) const { http://git-wip-us.apache.org/repos/asf/arrow/blob/cf1ac9cf/cpp/src/arrow/util/decimal.h ---------------------------------------------------------------------- diff --git a/cpp/src/arrow/util/decimal.h b/cpp/src/arrow/util/decimal.h index ff30d44..a0dea09 100644 --- a/cpp/src/arrow/util/decimal.h +++ b/cpp/src/arrow/util/decimal.h @@ -104,17 +104,15 @@ class ARROW_EXPORT Decimal128 { /// \brief Get the low bits of the two's complement representation of the number. uint64_t low_bits() const { return low_bits_; } - /// \brief Put the raw bytes of the value into a pointer to uint8_t. - Status ToBytes(std::array<uint8_t, 16>* out) const; + /// \brief Return the raw bytes of the value. + std::array<uint8_t, 16> ToBytes() const; /// \brief Convert the Decimal128 value to a base 10 decimal string with the given - /// precision - /// and scale. + /// precision and scale. std::string ToString(int precision, int scale) const; /// \brief Convert a decimal string to an Decimal128 value, optionally including - /// precision - /// and scale if they're passed in and not null. + /// precision and scale if they're passed in and not null. static Status FromString(const std::string& s, Decimal128* out, int* precision = nullptr, int* scale = nullptr);