Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: [C++] Get rid of code duplication in Decimal##bit_width #8417

Closed
wants to merge 6 commits into from
Closed

WIP: [C++] Get rid of code duplication in Decimal##bit_width #8417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4b87148
Get rid of code duplication in Decimal
dchigarev Oct 9, 2020
c64085e
Type id fix
dchigarev Oct 9, 2020
f2b25ce
Typo fix (class -> struct)
dchigarev Oct 9, 2020
6d13ab0
Get rid of declaring API via '#define'
dchigarev Oct 12, 2020
17b0e31
Get rid of code duplication at DecimalScalar
dchigarev Oct 12, 2020
4346e4e
Typo fix
dchigarev Oct 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 9 additions & 20 deletions cpp/src/arrow/array/array_decimal.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,32 +32,21 @@ namespace arrow {

using internal::checked_cast;

// ----------------------------------------------------------------------
// Decimal128

Decimal128Array::Decimal128Array(const std::shared_ptr<ArrayData>& data)
template<uint32_t width>
BaseDecimalArray<width>::BaseDecimalArray(const std::shared_ptr<ArrayData>& data)
: FixedSizeBinaryArray(data) {
ARROW_CHECK_EQ(data->type->id(), Type::DECIMAL128);
ARROW_CHECK_EQ(data->type->id(), DecimalTypeTraits<width>::Id);
}

std::string Decimal128Array::FormatValue(int64_t i) const {
const auto& type_ = checked_cast<const Decimal128Type&>(*type());
const Decimal128 value(GetValue(i));
template<uint32_t width>
std::string BaseDecimalArray<width>::FormatValue(int64_t i) const {
const auto& type_ = checked_cast<const typename DecimalTypeTraits<width>::TypeClass&>(*type());
const typename DecimalTypeTraits<width>::ValueType value(GetValue(i));
return value.ToString(type_.scale());
}

// ----------------------------------------------------------------------
// Decimal256

Decimal256Array::Decimal256Array(const std::shared_ptr<ArrayData>& data)
: FixedSizeBinaryArray(data) {
ARROW_CHECK_EQ(data->type->id(), Type::DECIMAL256);
}

std::string Decimal256Array::FormatValue(int64_t i) const {
const auto& type_ = checked_cast<const Decimal256Type&>(*type());
const Decimal256 value(GetValue(i));
return value.ToString(type_.scale());
}
template class BaseDecimalArray<128>;
template class BaseDecimalArray<256>;

} // namespace arrow
40 changes: 17 additions & 23 deletions cpp/src/arrow/array/array_decimal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,45 +22,39 @@
#include <string>

#include "arrow/array/array_binary.h"
#include "arrow/util/decimal_type_traits.h"
#include "arrow/array/data.h"
#include "arrow/type.h"
#include "arrow/util/visibility.h"

namespace arrow {

// ----------------------------------------------------------------------
// Decimal128Array

/// Concrete Array class for 128-bit decimal data
class ARROW_EXPORT Decimal128Array : public FixedSizeBinaryArray {
/// Template Array class for decimal data
template<uint32_t width>
class BaseDecimalArray : public FixedSizeBinaryArray {
public:
using TypeClass = Decimal128Type;
using TypeClass = typename DecimalTypeTraits<width>::TypeClass;

using FixedSizeBinaryArray::FixedSizeBinaryArray;

/// \brief Construct Decimal128Array from ArrayData instance
explicit Decimal128Array(const std::shared_ptr<ArrayData>& data);
/// \brief Construct DecimalArray from ArrayData instance
explicit BaseDecimalArray(const std::shared_ptr<ArrayData>& data);

std::string FormatValue(int64_t i) const;
};

// Backward compatibility
using DecimalArray = Decimal128Array;

// ----------------------------------------------------------------------
// Decimal256Array

/// Concrete Array class for 256-bit decimal data
class ARROW_EXPORT Decimal256Array : public FixedSizeBinaryArray {
public:
using TypeClass = Decimal256Type;
/// Array class for decimal 128-bit data
class ARROW_EXPORT Decimal128Array : public BaseDecimalArray<128> {
using BaseDecimalArray<128>::BaseDecimalArray;
};

using FixedSizeBinaryArray::FixedSizeBinaryArray;
/// Array class for decimal 256-bit data
class ARROW_EXPORT Decimal256Array : public BaseDecimalArray<256> {
using BaseDecimalArray<256>::BaseDecimalArray;
};

/// \brief Construct Decimal256Array from ArrayData instance
explicit Decimal256Array(const std::shared_ptr<ArrayData>& data);
// Backward compatibility
using DecimalArray = Decimal128Array;

std::string FormatValue(int64_t i) const;
};

} // namespace arrow
58 changes: 16 additions & 42 deletions cpp/src/arrow/array/builder_decimal.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,30 +33,35 @@ class Buffer;
class MemoryPool;

// ----------------------------------------------------------------------
// Decimal128Builder
// BaseDecimalBuilder

Decimal128Builder::Decimal128Builder(const std::shared_ptr<DataType>& type,
template<uint32_t width>
BaseDecimalBuilder<width>::BaseDecimalBuilder(const std::shared_ptr<DataType>& type,
MemoryPool* pool)
: FixedSizeBinaryBuilder(type, pool),
decimal_type_(internal::checked_pointer_cast<Decimal128Type>(type)) {}
decimal_type_(internal::checked_pointer_cast<typename DecimalTypeTraits<width>::TypeClass>(type)) {}

Status Decimal128Builder::Append(Decimal128 value) {
template<uint32_t width>
Status BaseDecimalBuilder<width>::Append(typename DecimalTypeTraits<width>::ValueType value) {
RETURN_NOT_OK(FixedSizeBinaryBuilder::Reserve(1));
UnsafeAppend(value);
return Status::OK();
}

void Decimal128Builder::UnsafeAppend(Decimal128 value) {
template<uint32_t width>
void BaseDecimalBuilder<width>::UnsafeAppend(typename DecimalTypeTraits<width>::ValueType value) {
value.ToBytes(GetMutableValue(length()));
byte_builder_.UnsafeAdvance(16);
byte_builder_.UnsafeAdvance((width >> 3));
UnsafeAppendToBitmap(true);
}

void Decimal128Builder::UnsafeAppend(util::string_view value) {
template<uint32_t width>
void BaseDecimalBuilder<width>::UnsafeAppend(util::string_view value) {
FixedSizeBinaryBuilder::UnsafeAppend(value);
}

Status Decimal128Builder::FinishInternal(std::shared_ptr<ArrayData>* out) {
template<uint32_t width>
Status BaseDecimalBuilder<width>::FinishInternal(std::shared_ptr<ArrayData>* out) {
std::shared_ptr<Buffer> data;
RETURN_NOT_OK(byte_builder_.Finish(&data));
std::shared_ptr<Buffer> null_bitmap;
Expand All @@ -67,39 +72,8 @@ Status Decimal128Builder::FinishInternal(std::shared_ptr<ArrayData>* out) {
return Status::OK();
}

// ----------------------------------------------------------------------
// Decimal256Builder

Decimal256Builder::Decimal256Builder(const std::shared_ptr<DataType>& type,
MemoryPool* pool)
: FixedSizeBinaryBuilder(type, pool),
decimal_type_(internal::checked_pointer_cast<Decimal256Type>(type)) {}

Status Decimal256Builder::Append(Decimal256 value) {
RETURN_NOT_OK(FixedSizeBinaryBuilder::Reserve(1));
UnsafeAppend(value);
return Status::OK();
}

void Decimal256Builder::UnsafeAppend(Decimal256 value) {
value.ToBytes(GetMutableValue(length()));
byte_builder_.UnsafeAdvance(32);
UnsafeAppendToBitmap(true);
}

void Decimal256Builder::UnsafeAppend(util::string_view value) {
FixedSizeBinaryBuilder::UnsafeAppend(value);
}

Status Decimal256Builder::FinishInternal(std::shared_ptr<ArrayData>* out) {
std::shared_ptr<Buffer> data;
RETURN_NOT_OK(byte_builder_.Finish(&data));
std::shared_ptr<Buffer> null_bitmap;
RETURN_NOT_OK(null_bitmap_builder_.Finish(&null_bitmap));

*out = ArrayData::Make(type(), length_, {null_bitmap, data}, null_count_);
capacity_ = length_ = null_count_ = 0;
return Status::OK();
}
// Instantiate template classes to have methods of it
template class BaseDecimalBuilder<128>;
template class BaseDecimalBuilder<256>;

} // namespace arrow
54 changes: 19 additions & 35 deletions cpp/src/arrow/array/builder_decimal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,27 @@
#include "arrow/array/builder_base.h"
#include "arrow/array/builder_binary.h"
#include "arrow/array/data.h"
#include "arrow/util/decimal_type_traits.h"
#include "arrow/status.h"
#include "arrow/type.h"
#include "arrow/util/visibility.h"

namespace arrow {

class ARROW_EXPORT Decimal128Builder : public FixedSizeBinaryBuilder {
public:
using TypeClass = Decimal128Type;
template<uint32_t width>
class BaseDecimalBuilder : public FixedSizeBinaryBuilder {
public:
using TypeClass = typename DecimalTypeTraits<width>::TypeClass;

explicit Decimal128Builder(const std::shared_ptr<DataType>& type,
MemoryPool* pool = default_memory_pool());
explicit BaseDecimalBuilder(const std::shared_ptr<DataType>& type,
MemoryPool* pool = default_memory_pool());

using FixedSizeBinaryBuilder::Append;
using FixedSizeBinaryBuilder::AppendValues;
using FixedSizeBinaryBuilder::Reset;

Status Append(Decimal128 val);
void UnsafeAppend(Decimal128 val);
Status Append(typename DecimalTypeTraits<width>::ValueType val);
void UnsafeAppend(typename DecimalTypeTraits<width>::ValueType val);
void UnsafeAppend(util::string_view val);

Status FinishInternal(std::shared_ptr<ArrayData>* out) override;
Expand All @@ -50,43 +52,25 @@ class ARROW_EXPORT Decimal128Builder : public FixedSizeBinaryBuilder {
using ArrayBuilder::Finish;
/// \endcond

Status Finish(std::shared_ptr<Decimal128Array>* out) { return FinishTyped(out); }
Status Finish(std::shared_ptr<typename DecimalTypeTraits<width>::ArrayType>* out) { return FinishTyped(out); }

std::shared_ptr<DataType> type() const override { return decimal_type_; }

protected:
std::shared_ptr<Decimal128Type> decimal_type_;
std::shared_ptr<typename DecimalTypeTraits<width>::TypeClass> decimal_type_;
};

class ARROW_EXPORT Decimal256Builder : public FixedSizeBinaryBuilder {
public:
using TypeClass = Decimal256Type;

explicit Decimal256Builder(const std::shared_ptr<DataType>& type,
MemoryPool* pool = default_memory_pool());

using FixedSizeBinaryBuilder::Append;
using FixedSizeBinaryBuilder::AppendValues;
using FixedSizeBinaryBuilder::Reset;

Status Append(Decimal256 val);
void UnsafeAppend(Decimal256 val);
void UnsafeAppend(util::string_view val);

Status FinishInternal(std::shared_ptr<ArrayData>* out) override;

/// \cond FALSE
using ArrayBuilder::Finish;
/// \endcond

Status Finish(std::shared_ptr<Decimal256Array>* out) { return FinishTyped(out); }

std::shared_ptr<DataType> type() const override { return decimal_type_; }
/// Builder class for decimal 128-bit
class ARROW_EXPORT Decimal128Builder : public BaseDecimalBuilder<128> {
using BaseDecimalBuilder<128>::BaseDecimalBuilder;
};

protected:
std::shared_ptr<Decimal256Type> decimal_type_;
/// Builder class for decimal 128-bit
class ARROW_EXPORT Decimal256Builder : public BaseDecimalBuilder<256> {
using BaseDecimalBuilder<256>::BaseDecimalBuilder;
};

// Backward compatibility
using DecimalBuilder = Decimal128Builder;

} // namespace arrow
25 changes: 12 additions & 13 deletions cpp/src/arrow/scalar.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "arrow/type_traits.h"
#include "arrow/util/compare.h"
#include "arrow/util/decimal.h"
#include "arrow/util/decimal_type_traits.h"
#include "arrow/util/string_view.h"
#include "arrow/util/visibility.h"

Expand Down Expand Up @@ -336,26 +337,24 @@ struct ARROW_EXPORT DurationScalar : public TemporalScalar<DurationType> {
using TemporalScalar<DurationType>::TemporalScalar;
};

struct ARROW_EXPORT Decimal128Scalar : public Scalar {
template<uint32_t width>
struct BaseDecimalScalar : public Scalar {
using Scalar::Scalar;
using TypeClass = Decimal128Type;
using ValueType = Decimal128;
using TypeClass = typename DecimalTypeTraits<width>::TypeClass;
using ValueType = typename DecimalTypeTraits<width>::ValueType;

Decimal128Scalar(Decimal128 value, std::shared_ptr<DataType> type)
BaseDecimalScalar(ValueType value, std::shared_ptr<DataType> type)
: Scalar(std::move(type), true), value(value) {}

Decimal128 value;
ValueType value;
};

struct ARROW_EXPORT Decimal256Scalar : public Scalar {
using Scalar::Scalar;
using TypeClass = Decimal256Type;
using ValueType = Decimal256;

Decimal256Scalar(Decimal256 value, std::shared_ptr<DataType> type)
: Scalar(std::move(type), true), value(value) {}
struct ARROW_EXPORT Decimal128Scalar : public BaseDecimalScalar<128> {
using BaseDecimalScalar<128>::BaseDecimalScalar;
};

Decimal256 value;
struct ARROW_EXPORT Decimal256Scalar : public BaseDecimalScalar<256> {
using BaseDecimalScalar<256>::BaseDecimalScalar;
};

struct ARROW_EXPORT BaseListScalar : public Scalar {
Expand Down
40 changes: 13 additions & 27 deletions cpp/src/arrow/type.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
#include "arrow/util/make_unique.h"
#include "arrow/util/range.h"
#include "arrow/util/vector.h"
#include "arrow/util/decimal_type_traits.h"
#include "arrow/visitor_inline.h"

namespace arrow {
Expand Down Expand Up @@ -748,35 +749,22 @@ std::vector<std::shared_ptr<Field>> StructType::GetAllFieldsByName(
}

// ----------------------------------------------------------------------
// Decimal128 type
// Decimal type

Decimal128Type::Decimal128Type(int32_t precision, int32_t scale)
: DecimalType(type_id, 16, precision, scale) {
ARROW_CHECK_GE(precision, kMinPrecision);
ARROW_CHECK_LE(precision, kMaxPrecision);
}

Result<std::shared_ptr<DataType>> Decimal128Type::Make(int32_t precision, int32_t scale) {
if (precision < kMinPrecision || precision > kMaxPrecision) {
return Status::Invalid("Decimal precision out of range: ", precision);
}
return std::make_shared<Decimal128Type>(precision, scale);
}

// ----------------------------------------------------------------------
// Decimal256 type

Decimal256Type::Decimal256Type(int32_t precision, int32_t scale)
: DecimalType(type_id, 32, precision, scale) {
template<uint32_t width>
BaseDecimalType<width>::BaseDecimalType(int32_t precision, int32_t scale)
: DecimalType(DecimalTypeTraits<width>::Id, (width >> 3), precision, scale) {
ARROW_CHECK_GE(precision, kMinPrecision);
ARROW_CHECK_LE(precision, kMaxPrecision);
}

Result<std::shared_ptr<DataType>> Decimal256Type::Make(int32_t precision, int32_t scale) {
template<uint32_t width>
Result<std::shared_ptr<DataType>> BaseDecimalType<width>::Make(int32_t precision, int32_t scale) {
if (precision < kMinPrecision || precision > kMaxPrecision) {
return Status::Invalid("Decimal precision out of range: ", precision);
}
return std::make_shared<Decimal256Type>(precision, scale);
return std::make_shared<typename DecimalTypeTraits<width>::TypeClass>(precision, scale);
}

// ----------------------------------------------------------------------
Expand Down Expand Up @@ -2169,16 +2157,14 @@ std::shared_ptr<DataType> decimal256(int32_t precision, int32_t scale) {
return std::make_shared<Decimal256Type>(precision, scale);
}

std::string Decimal128Type::ToString() const {
template<uint32_t width>
std::string BaseDecimalType<width>::ToString() const {
std::stringstream s;
s << "decimal(" << precision_ << ", " << scale_ << ")";
s << type_name() << "(" << precision_ << ", " << scale_ << ")";
return s.str();
}

std::string Decimal256Type::ToString() const {
std::stringstream s;
s << "decimal256(" << precision_ << ", " << scale_ << ")";
return s.str();
}
template class BaseDecimalType<128>;
template class BaseDecimalType<256>;

} // namespace arrow
Loading