Skip to content

Commit

Permalink
Don't use virtual functions in constructor directly
Browse files Browse the repository at this point in the history
  • Loading branch information
@kou
kou committed Jul 13, 2024
1 parent e58ce08 commit 5bf9935
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 81 deletions.
28 changes: 16 additions & 12 deletions cpp/src/arrow/array/array_base.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,24 +240,26 @@ class ARROW_EXPORT Array {

protected:
Array() = default;
explicit Array(const std::shared_ptr<ArrayData>& data,
const std::shared_ptr<ArrayStatistics>& statistics = NULLPTR) {
ARROW_DEFAULT_MOVE_AND_ASSIGN(Array);

std::shared_ptr<ArrayData> data_;
const uint8_t* null_bitmap_data_ = NULLPTR;

/// Protected method for constructors
void Init(const std::shared_ptr<ArrayData>& data,
const std::shared_ptr<ArrayStatistics>& statistics) {
ValidateData(data);
SetData(data);
if (statistics) {
SetStatistics(statistics);
}
}
ARROW_DEFAULT_MOVE_AND_ASSIGN(Array);

std::shared_ptr<ArrayData> data_;
const uint8_t* null_bitmap_data_ = NULLPTR;

/// Protected method for constructors
virtual void ValidateData(const std::shared_ptr<ArrayData>& data) {}

/// Protected method for constructors
virtual void SetData(const std::shared_ptr<ArrayData>& data) {
ValidateData(data);
if (data->buffers.size() > 0) {
null_bitmap_data_ = data->GetValuesSafe<uint8_t>(0, /*offset=*/0);
} else {
Expand Down Expand Up @@ -306,13 +308,14 @@ class ARROW_EXPORT PrimitiveArray : public FlatArray {
PrimitiveArray() : raw_values_(NULLPTR) {}

void SetData(const std::shared_ptr<ArrayData>& data) override {
this->Array::SetData(data);
Array::SetData(data);
raw_values_ = data->GetValuesSafe<uint8_t>(1, /*offset=*/0);
}

explicit PrimitiveArray(const std::shared_ptr<ArrayData>& data,
const std::shared_ptr<ArrayStatistics>& statistics = NULLPTR)
: FlatArray(data, statistics) {}
const std::shared_ptr<ArrayStatistics>& statistics = NULLPTR) {
Init(data, statistics);
}

const uint8_t* raw_values_;
};
Expand All @@ -323,8 +326,9 @@ class ARROW_EXPORT NullArray : public FlatArray {
using TypeClass = NullType;

explicit NullArray(const std::shared_ptr<ArrayData>& data,
const std::shared_ptr<ArrayStatistics>& statistics = NULLPTR)
: FlatArray(data, statistics) {}
const std::shared_ptr<ArrayStatistics>& statistics = NULLPTR) {
Init(data, statistics);
}
explicit NullArray(int64_t length);

private:
Expand Down
53 changes: 30 additions & 23 deletions cpp/src/arrow/array/array_binary.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,10 @@ class BaseBinaryArray : public FlatArray {
protected:
// For subclasses
BaseBinaryArray() = default;
explicit BaseBinaryArray(const std::shared_ptr<ArrayData>& data,
const std::shared_ptr<ArrayStatistics>& statistics = NULLPTR)
: FlatArray(data, statistics) {}

// Protected method for constructors
void SetData(const std::shared_ptr<ArrayData>& data) override {
this->Array::SetData(data);
Array::SetData(data);
raw_value_offsets_ = data->GetValuesSafe<offset_type>(1, /*offset=*/0);
raw_data_ = data->GetValuesSafe<uint8_t>(2, /*offset=*/0);
}
Expand All @@ -159,8 +156,9 @@ class BaseBinaryArray : public FlatArray {
class ARROW_EXPORT BinaryArray : public BaseBinaryArray<BinaryType> {
public:
explicit BinaryArray(const std::shared_ptr<ArrayData>& data,
const std::shared_ptr<ArrayStatistics>& statistics = NULLPTR)
: BaseBinaryArray(data, statistics) {}
const std::shared_ptr<ArrayStatistics>& statistics = NULLPTR) {
Init(data, statistics);
}

BinaryArray(int64_t length, const std::shared_ptr<Buffer>& value_offsets,
const std::shared_ptr<Buffer>& data,
Expand All @@ -180,8 +178,9 @@ class ARROW_EXPORT StringArray : public BinaryArray {
using TypeClass = StringType;

explicit StringArray(const std::shared_ptr<ArrayData>& data,
const std::shared_ptr<ArrayStatistics>& statistics = NULLPTR)
: BinaryArray(data, statistics) {}
const std::shared_ptr<ArrayStatistics>& statistics = NULLPTR) {
Init(data, statistics);
}

StringArray(int64_t length, const std::shared_ptr<Buffer>& value_offsets,
const std::shared_ptr<Buffer>& data,
Expand All @@ -200,9 +199,11 @@ class ARROW_EXPORT StringArray : public BinaryArray {
/// Concrete Array class for large variable-size binary data
class ARROW_EXPORT LargeBinaryArray : public BaseBinaryArray<LargeBinaryType> {
public:
explicit LargeBinaryArray(const std::shared_ptr<ArrayData>& data,
const std::shared_ptr<ArrayStatistics>& statistics = NULLPTR)
: BaseBinaryArray(data, statistics) {}
explicit LargeBinaryArray(
const std::shared_ptr<ArrayData>& data,
const std::shared_ptr<ArrayStatistics>& statistics = NULLPTR) {
Init(data, statistics);
}

LargeBinaryArray(int64_t length, const std::shared_ptr<Buffer>& value_offsets,
const std::shared_ptr<Buffer>& data,
Expand All @@ -220,9 +221,11 @@ class ARROW_EXPORT LargeStringArray : public LargeBinaryArray {
public:
using TypeClass = LargeStringType;

explicit LargeStringArray(const std::shared_ptr<ArrayData>& data,
const std::shared_ptr<ArrayStatistics>& statistics = NULLPTR)
: LargeBinaryArray(data, statistics) {}
explicit LargeStringArray(
const std::shared_ptr<ArrayData>& data,
const std::shared_ptr<ArrayStatistics>& statistics = NULLPTR) {
Init(data, statistics);
}

LargeStringArray(int64_t length, const std::shared_ptr<Buffer>& value_offsets,
const std::shared_ptr<Buffer>& data,
Expand Down Expand Up @@ -250,8 +253,9 @@ class ARROW_EXPORT BinaryViewArray : public FlatArray {
using c_type = BinaryViewType::c_type;

explicit BinaryViewArray(const std::shared_ptr<ArrayData>& data,
const std::shared_ptr<ArrayStatistics>& statistics = NULLPTR)
: FlatArray(data, statistics) {}
const std::shared_ptr<ArrayStatistics>& statistics = NULLPTR) {
Init(data, statistics);
}

BinaryViewArray(std::shared_ptr<DataType> type, int64_t length,
std::shared_ptr<Buffer> views, BufferVector data_buffers,
Expand All @@ -273,7 +277,8 @@ class ARROW_EXPORT BinaryViewArray : public FlatArray {
IteratorType end() const { return IteratorType(*this, length()); }

protected:
using FlatArray::FlatArray;
// This constructor defers Init() to a derived array class
BinaryViewArray() = default;

void ValidateData(const std::shared_ptr<ArrayData>& data) override;

Expand All @@ -292,8 +297,9 @@ class ARROW_EXPORT StringViewArray : public BinaryViewArray {
using TypeClass = StringViewType;

explicit StringViewArray(const std::shared_ptr<ArrayData>& data,
const std::shared_ptr<ArrayStatistics>& statistics = NULLPTR)
: BinaryViewArray(data, statistics) {}
const std::shared_ptr<ArrayStatistics>& statistics = NULLPTR) {
Init(data, statistics);
}

using BinaryViewArray::BinaryViewArray;

Expand All @@ -317,8 +323,9 @@ class ARROW_EXPORT FixedSizeBinaryArray : public PrimitiveArray {

explicit FixedSizeBinaryArray(
const std::shared_ptr<ArrayData>& data,
const std::shared_ptr<ArrayStatistics>& statistics = NULLPTR)
: PrimitiveArray(data, statistics) {}
const std::shared_ptr<ArrayStatistics>& statistics = NULLPTR) {
Init(data, statistics);
}

FixedSizeBinaryArray(const std::shared_ptr<DataType>& type, int64_t length,
const std::shared_ptr<Buffer>& data,
Expand Down Expand Up @@ -347,8 +354,8 @@ class ARROW_EXPORT FixedSizeBinaryArray : public PrimitiveArray {
IteratorType end() const { return IteratorType(*this, length()); }

protected:
void SetData(const std::shared_ptr<ArrayData>& data) {
this->PrimitiveArray::SetData(data);
void SetData(const std::shared_ptr<ArrayData>& data) override {
PrimitiveArray::SetData(data);
byte_width_ =
internal::checked_cast<const FixedSizeBinaryType&>(*type()).byte_width();
}
Expand Down
5 changes: 3 additions & 2 deletions cpp/src/arrow/array/array_dict.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,9 @@ int64_t DictionaryArray::GetValueIndex(int64_t i) const {

DictionaryArray::DictionaryArray(const std::shared_ptr<ArrayData>& data,
const std::shared_ptr<ArrayStatistics>& statistics)
: Array(data, statistics),
dict_type_(checked_cast<const DictionaryType*>(data->type.get())) {}
: dict_type_(checked_cast<const DictionaryType*>(data->type.get())) {
Init(data, statistics);
}

void DictionaryArray::ValidateData(const std::shared_ptr<ArrayData>& data) {
ARROW_CHECK_EQ(data->type->id(), Type::DICTIONARY);
Expand Down
8 changes: 4 additions & 4 deletions cpp/src/arrow/array/array_nested.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,7 @@ void FixedSizeListArray::ValidateData(const std::shared_ptr<ArrayData>& data) {
}

void FixedSizeListArray::SetData(const std::shared_ptr<ArrayData>& data) {
this->Array::SetData(data);
Array::SetData(data);

ARROW_CHECK_EQ(list_type()->value_type()->id(), data->child_data[0]->type->id());
DCHECK(list_type()->value_type()->Equals(data->child_data[0]->type));
Expand Down Expand Up @@ -1160,7 +1160,7 @@ Result<std::shared_ptr<Array>> StructArray::GetFlattenedField(int index,
// UnionArray

void UnionArray::SetData(const std::shared_ptr<ArrayData>& data) {
this->Array::SetData(data);
Array::SetData(data);

union_type_ = checked_cast<const UnionType*>(data_->type.get());

Expand All @@ -1170,7 +1170,7 @@ void UnionArray::SetData(const std::shared_ptr<ArrayData>& data) {
}

void SparseUnionArray::SetData(const std::shared_ptr<ArrayData>& data) {
this->UnionArray::SetData(data);
UnionArray::SetData(data);
ARROW_CHECK_EQ(data_->type->id(), Type::SPARSE_UNION);
ARROW_CHECK_EQ(data_->buffers.size(), 2);

Expand All @@ -1179,7 +1179,7 @@ void SparseUnionArray::SetData(const std::shared_ptr<ArrayData>& data) {
}

void DenseUnionArray::SetData(const std::shared_ptr<ArrayData>& data) {
this->UnionArray::SetData(data);
UnionArray::SetData(data);

ARROW_CHECK_EQ(data_->type->id(), Type::DENSE_UNION);
ARROW_CHECK_EQ(data_->buffers.size(), 3);
Expand Down
Loading

0 comments on commit 5bf9935

Please sign in to comment.