[
https://issues.apache.org/jira/browse/ARROW-1777?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16265783#comment-16265783
]
ASF GitHub Bot commented on ARROW-1777:
---------------------------------------
wesm closed pull request #1353: ARROW-1777: [C++] Add ArrayData::Make static
ctor for more convenient construction
URL: https://github.com/apache/arrow/pull/1353
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/cpp/src/arrow/array.cc b/cpp/src/arrow/array.cc
index 4ceb071ac..4b1fabfdc 100644
--- a/cpp/src/arrow/array.cc
+++ b/cpp/src/arrow/array.cc
@@ -36,6 +36,20 @@
namespace arrow {
+std::shared_ptr<ArrayData> ArrayData::Make(const std::shared_ptr<DataType>&
type,
+ int64_t length,
+
std::vector<std::shared_ptr<Buffer>>&& buffers,
+ int64_t null_count, int64_t offset)
{
+ return std::make_shared<ArrayData>(type, length, std::move(buffers),
null_count,
+ offset);
+}
+
+std::shared_ptr<ArrayData> ArrayData::Make(const std::shared_ptr<DataType>&
type,
+ int64_t length, int64_t null_count,
+ int64_t offset) {
+ return std::make_shared<ArrayData>(type, length, null_count, offset);
+}
+
// ----------------------------------------------------------------------
// Base array class
@@ -112,8 +126,7 @@ std::string Array::ToString() const {
}
NullArray::NullArray(int64_t length) {
- BufferVector buffers = {nullptr};
- SetData(std::make_shared<ArrayData>(null(), length, std::move(buffers),
length));
+ SetData(ArrayData::Make(null(), length, {nullptr}, length));
}
// ----------------------------------------------------------------------
@@ -123,9 +136,7 @@ PrimitiveArray::PrimitiveArray(const
std::shared_ptr<DataType>& type, int64_t le
const std::shared_ptr<Buffer>& data,
const std::shared_ptr<Buffer>& null_bitmap,
int64_t null_count, int64_t offset) {
- BufferVector buffers = {null_bitmap, data};
- SetData(
- std::make_shared<ArrayData>(type, length, std::move(buffers),
null_count, offset));
+ SetData(ArrayData::Make(type, length, {null_bitmap, data}, null_count,
offset));
}
const uint8_t* PrimitiveArray::raw_values() const {
@@ -165,9 +176,8 @@ ListArray::ListArray(const std::shared_ptr<DataType>& type,
int64_t length,
const std::shared_ptr<Array>& values,
const std::shared_ptr<Buffer>& null_bitmap, int64_t
null_count,
int64_t offset) {
- BufferVector buffers = {null_bitmap, value_offsets};
auto internal_data =
- std::make_shared<ArrayData>(type, length, std::move(buffers),
null_count, offset);
+ ArrayData::Make(type, length, {null_bitmap, value_offsets}, null_count,
offset);
internal_data->child_data.emplace_back(values->data());
SetData(internal_data);
}
@@ -219,9 +229,8 @@ Status ListArray::FromArrays(const Array& offsets, const
Array& values, MemoryPo
}
auto list_type = list(values.type());
- auto internal_data =
- std::make_shared<ArrayData>(list_type, num_offsets - 1,
std::move(buffers),
- offsets.null_count(), offsets.offset());
+ auto internal_data = ArrayData::Make(list_type, num_offsets - 1,
std::move(buffers),
+ offsets.null_count(), offsets.offset());
internal_data->child_data.push_back(values.data());
*out = std::make_shared<ListArray>(internal_data);
@@ -276,9 +285,8 @@ BinaryArray::BinaryArray(const std::shared_ptr<DataType>&
type, int64_t length,
const std::shared_ptr<Buffer>& data,
const std::shared_ptr<Buffer>& null_bitmap, int64_t
null_count,
int64_t offset) {
- BufferVector buffers = {null_bitmap, value_offsets, data};
- SetData(
- std::make_shared<ArrayData>(type, length, std::move(buffers),
null_count, offset));
+ SetData(ArrayData::Make(type, length, {null_bitmap, value_offsets, data},
null_count,
+ offset));
}
StringArray::StringArray(const std::shared_ptr<ArrayData>& data) {
@@ -338,9 +346,7 @@ StructArray::StructArray(const std::shared_ptr<DataType>&
type, int64_t length,
const std::vector<std::shared_ptr<Array>>& children,
std::shared_ptr<Buffer> null_bitmap, int64_t
null_count,
int64_t offset) {
- BufferVector buffers = {null_bitmap};
- SetData(
- std::make_shared<ArrayData>(type, length, std::move(buffers),
null_count, offset));
+ SetData(ArrayData::Make(type, length, {null_bitmap}, null_count, offset));
for (const auto& child : children) {
data_->child_data.push_back(child->data());
}
@@ -384,9 +390,8 @@ UnionArray::UnionArray(const std::shared_ptr<DataType>&
type, int64_t length,
const std::shared_ptr<Buffer>& value_offsets,
const std::shared_ptr<Buffer>& null_bitmap, int64_t
null_count,
int64_t offset) {
- BufferVector buffers = {null_bitmap, type_ids, value_offsets};
- auto internal_data =
- std::make_shared<ArrayData>(type, length, std::move(buffers),
null_count, offset);
+ auto internal_data = ArrayData::Make(
+ type, length, {null_bitmap, type_ids, value_offsets}, null_count,
offset);
for (const auto& child : children) {
internal_data->child_data.push_back(child->data());
}
@@ -416,9 +421,8 @@ Status UnionArray::MakeDense(const Array& type_ids, const
Array& value_offsets,
static_cast<const UInt8Array&>(type_ids).values(),
static_cast<const
Int32Array&>(value_offsets).values()};
auto union_type = union_(children, UnionMode::DENSE);
- auto internal_data =
- std::make_shared<ArrayData>(union_type, type_ids.length(),
std::move(buffers),
- type_ids.null_count(), type_ids.offset());
+ auto internal_data = ArrayData::Make(union_type, type_ids.length(),
std::move(buffers),
+ type_ids.null_count(),
type_ids.offset());
for (const auto& child : children) {
internal_data->child_data.push_back(child->data());
}
@@ -435,9 +439,8 @@ Status UnionArray::MakeSparse(const Array& type_ids,
BufferVector buffers = {type_ids.null_bitmap(),
static_cast<const UInt8Array&>(type_ids).values(),
nullptr};
auto union_type = union_(children, UnionMode::SPARSE);
- auto internal_data =
- std::make_shared<ArrayData>(union_type, type_ids.length(),
std::move(buffers),
- type_ids.null_count(), type_ids.offset());
+ auto internal_data = ArrayData::Make(union_type, type_ids.length(),
std::move(buffers),
+ type_ids.null_count(),
type_ids.offset());
for (const auto& child : children) {
internal_data->child_data.push_back(child->data());
if (child->length() != type_ids.length()) {
diff --git a/cpp/src/arrow/array.h b/cpp/src/arrow/array.h
index dda9dd38b..ec5381d6e 100644
--- a/cpp/src/arrow/array.h
+++ b/cpp/src/arrow/array.h
@@ -104,6 +104,17 @@ struct ARROW_EXPORT ArrayData {
this->buffers = std::move(buffers);
}
+ static std::shared_ptr<ArrayData> Make(const std::shared_ptr<DataType>& type,
+ int64_t length,
+
std::vector<std::shared_ptr<Buffer>>&& buffers,
+ int64_t null_count =
kUnknownNullCount,
+ int64_t offset = 0);
+
+ static std::shared_ptr<ArrayData> Make(const std::shared_ptr<DataType>& type,
+ int64_t length,
+ int64_t null_count =
kUnknownNullCount,
+ int64_t offset = 0);
+
// Move constructor
ArrayData(ArrayData&& other) noexcept
: type(std::move(other.type)),
diff --git a/cpp/src/arrow/builder.cc b/cpp/src/arrow/builder.cc
index a42f90245..4d7fd5ff8 100644
--- a/cpp/src/arrow/builder.cc
+++ b/cpp/src/arrow/builder.cc
@@ -219,8 +219,7 @@ void ArrayBuilder::UnsafeSetNotNull(int64_t length) {
// Null builder
Status NullBuilder::FinishInternal(std::shared_ptr<ArrayData>* out) {
- BufferVector buffers = {nullptr};
- *out = std::make_shared<ArrayData>(null(), length_, std::move(buffers),
length_);
+ *out = ArrayData::Make(null(), length_, {nullptr}, length_);
length_ = null_count_ = 0;
return Status::OK();
}
@@ -314,8 +313,7 @@ Status
PrimitiveBuilder<T>::FinishInternal(std::shared_ptr<ArrayData>* out) {
// Trim buffers
RETURN_NOT_OK(data_->Resize(bytes_required));
}
- BufferVector buffers = {null_bitmap_, data_};
- *out = std::make_shared<ArrayData>(type_, length_, std::move(buffers),
null_count_);
+ *out = ArrayData::Make(type_, length_, {null_bitmap_, data_}, null_count_);
data_ = null_bitmap_ = nullptr;
capacity_ = length_ = null_count_ = 0;
@@ -404,9 +402,7 @@ Status
AdaptiveIntBuilder::FinishInternal(std::shared_ptr<ArrayData>* out) {
return Status::NotImplemented("Only ints of size 1,2,4,8 are supported");
}
- BufferVector buffers = {null_bitmap_, data_};
- *out =
- std::make_shared<ArrayData>(output_type, length_, std::move(buffers),
null_count_);
+ *out = ArrayData::Make(output_type, length_, {null_bitmap_, data_},
null_count_);
data_ = null_bitmap_ = nullptr;
capacity_ = length_ = null_count_ = 0;
@@ -562,9 +558,7 @@ Status
AdaptiveUIntBuilder::FinishInternal(std::shared_ptr<ArrayData>* out) {
return Status::NotImplemented("Only ints of size 1,2,4,8 are supported");
}
- BufferVector buffers = {null_bitmap_, data_};
- *out =
- std::make_shared<ArrayData>(output_type, length_, std::move(buffers),
null_count_);
+ *out = ArrayData::Make(output_type, length_, {null_bitmap_, data_},
null_count_);
data_ = null_bitmap_ = nullptr;
capacity_ = length_ = null_count_ = 0;
@@ -741,8 +735,7 @@ Status
BooleanBuilder::FinishInternal(std::shared_ptr<ArrayData>* out) {
// Trim buffers
RETURN_NOT_OK(data_->Resize(bytes_required));
}
- BufferVector buffers = {null_bitmap_, data_};
- *out = std::make_shared<ArrayData>(boolean(), length_, std::move(buffers),
null_count_);
+ *out = ArrayData::Make(boolean(), length_, {null_bitmap_, data_},
null_count_);
data_ = null_bitmap_ = nullptr;
capacity_ = length_ = null_count_ = 0;
@@ -828,8 +821,7 @@ Status
Decimal128Builder::FinishInternal(std::shared_ptr<ArrayData>* out) {
std::shared_ptr<Buffer> data;
RETURN_NOT_OK(byte_builder_.Finish(&data));
- BufferVector buffers = {null_bitmap_, data};
- *out = std::make_shared<ArrayData>(type_, length_, std::move(buffers),
null_count_);
+ *out = ArrayData::Make(type_, length_, {null_bitmap_, data}, null_count_);
return Status::OK();
}
@@ -896,8 +888,7 @@ Status
ListBuilder::FinishInternal(std::shared_ptr<ArrayData>* out) {
RETURN_NOT_OK(value_builder_->FinishInternal(&items));
}
- BufferVector buffers = {null_bitmap_, offsets};
- *out = std::make_shared<ArrayData>(type_, length_, std::move(buffers),
null_count_);
+ *out = ArrayData::Make(type_, length_, {null_bitmap_, offsets}, null_count_);
(*out)->child_data.emplace_back(std::move(items));
Reset();
return Status::OK();
@@ -969,8 +960,8 @@ Status
BinaryBuilder::FinishInternal(std::shared_ptr<ArrayData>* out) {
RETURN_NOT_OK(offsets_builder_.Finish(&offsets));
RETURN_NOT_OK(value_data_builder_.Finish(&value_data));
- BufferVector buffers = {null_bitmap_, offsets, value_data};
- *out = std::make_shared<ArrayData>(type_, length_, std::move(buffers),
null_count_, 0);
+ *out = ArrayData::Make(type_, length_, {null_bitmap_, offsets, value_data},
null_count_,
+ 0);
Reset();
return Status::OK();
}
@@ -1040,8 +1031,7 @@ Status
FixedSizeBinaryBuilder::FinishInternal(std::shared_ptr<ArrayData>* out) {
std::shared_ptr<Buffer> data;
RETURN_NOT_OK(byte_builder_.Finish(&data));
- BufferVector buffers = {null_bitmap_, data};
- *out = std::make_shared<ArrayData>(type_, length_, std::move(buffers),
null_count_);
+ *out = ArrayData::Make(type_, length_, {null_bitmap_, data}, null_count_);
return Status::OK();
}
@@ -1060,8 +1050,7 @@ StructBuilder::StructBuilder(const
std::shared_ptr<DataType>& type, MemoryPool*
}
Status StructBuilder::FinishInternal(std::shared_ptr<ArrayData>* out) {
- BufferVector buffers = {null_bitmap_};
- *out = std::make_shared<ArrayData>(type_, length_, std::move(buffers),
null_count_);
+ *out = ArrayData::Make(type_, length_, {null_bitmap_}, null_count_);
(*out)->child_data.resize(field_builders_.size());
for (size_t i = 0; i < field_builders_.size(); ++i) {
diff --git a/cpp/src/arrow/compute/compute-test.cc
b/cpp/src/arrow/compute/compute-test.cc
index 96edd8f01..d5158978c 100644
--- a/cpp/src/arrow/compute/compute-test.cc
+++ b/cpp/src/arrow/compute/compute-test.cc
@@ -688,7 +688,7 @@ TEST_F(TestCast, PreallocatedMemory) {
std::unique_ptr<UnaryKernel> kernel;
ASSERT_OK(GetCastFunction(*int32(), out_type, options, &kernel));
- auto out_data = std::make_shared<ArrayData>(out_type, length);
+ auto out_data = ArrayData::Make(out_type, length);
shared_ptr<Buffer> out_values;
ASSERT_OK(this->ctx_.Allocate(length * sizeof(int64_t), &out_values));
diff --git a/cpp/src/arrow/compute/kernels/cast.cc
b/cpp/src/arrow/compute/kernels/cast.cc
index d595d2ea5..d48d66992 100644
--- a/cpp/src/arrow/compute/kernels/cast.cc
+++ b/cpp/src/arrow/compute/kernels/cast.cc
@@ -747,7 +747,7 @@ class CastKernel : public UnaryKernel {
ArrayData* result;
if (out->kind() == Datum::NONE) {
- out->value = std::make_shared<ArrayData>(out_type_, in_data.length);
+ out->value = ArrayData::Make(out_type_, in_data.length);
}
result = out->array().get();
diff --git a/cpp/src/arrow/compute/kernels/hash.cc
b/cpp/src/arrow/compute/kernels/hash.cc
index e47759d4d..66c907369 100644
--- a/cpp/src/arrow/compute/kernels/hash.cc
+++ b/cpp/src/arrow/compute/kernels/hash.cc
@@ -342,8 +342,7 @@ class HashTableKernel<Type, Action,
enable_if_has_c_type<Type>> : public HashTab
auto dict_data = dict_.buffer;
RETURN_NOT_OK(dict_data->Resize(dict_.size * sizeof(T), false));
- BufferVector buffers = {nullptr, dict_data};
- *out = std::make_shared<ArrayData>(type_, dict_.size, std::move(buffers),
0);
+ *out = ArrayData::Make(type_, dict_.size, {nullptr, dict_data}, 0);
return Status::OK();
}
@@ -528,7 +527,7 @@ class HashTableKernel<Type, Action, enable_if_binary<Type>>
: public HashTable {
RETURN_NOT_OK(dict_offsets_.Finish(&buffers[1]));
RETURN_NOT_OK(dict_data_.Finish(&buffers[2]));
- *out = std::make_shared<ArrayData>(type_, dict_size_, std::move(buffers),
0);
+ *out = ArrayData::Make(type_, dict_size_, std::move(buffers), 0);
return Status::OK();
}
@@ -634,7 +633,7 @@ class HashTableKernel<Type, Action,
enable_if_fixed_size_binary<Type>>
BufferVector buffers = {nullptr, nullptr};
RETURN_NOT_OK(dict_data_.Finish(&buffers[1]));
- *out = std::make_shared<ArrayData>(type_, dict_size_, std::move(buffers),
0);
+ *out = ArrayData::Make(type_, dict_size_, std::move(buffers), 0);
return Status::OK();
}
diff --git a/cpp/src/arrow/ipc/feather-test.cc
b/cpp/src/arrow/ipc/feather-test.cc
index e3de17f1f..8ec3b0e4a 100644
--- a/cpp/src/arrow/ipc/feather-test.cc
+++ b/cpp/src/arrow/ipc/feather-test.cc
@@ -366,15 +366,14 @@ TEST_F(TestTableWriter, TimeTypes) {
ArrayFromVector<Date32Type, int32_t>(is_valid, date_values_vec, &date_array);
const auto& prim_values = static_cast<const PrimitiveArray&>(*values);
- std::vector<std::shared_ptr<Buffer>> buffers = {prim_values.null_bitmap(),
- prim_values.values()};
+ BufferVector buffers = {prim_values.null_bitmap(), prim_values.values()};
std::vector<std::shared_ptr<ArrayData>> arrays;
arrays.push_back(date_array->data());
for (int i = 1; i < schema->num_fields(); ++i) {
- arrays.emplace_back(std::make_shared<ArrayData>(
- schema->field(i)->type(), values->length(), buffers,
values->null_count(), 0));
+ arrays.emplace_back(ArrayData::Make(schema->field(i)->type(),
values->length(),
+ BufferVector(buffers),
values->null_count(), 0));
}
auto batch = RecordBatch::Make(schema, values->length(), std::move(arrays));
diff --git a/cpp/src/arrow/ipc/feather.cc b/cpp/src/arrow/ipc/feather.cc
index 077dc3930..d339449c0 100644
--- a/cpp/src/arrow/ipc/feather.cc
+++ b/cpp/src/arrow/ipc/feather.cc
@@ -371,7 +371,7 @@ class TableReader::TableReaderImpl {
buffers.push_back(SliceBuffer(buffer, offset, buffer->size() - offset));
auto arr_data =
- std::make_shared<ArrayData>(type, meta->length(), buffers,
meta->null_count());
+ ArrayData::Make(type, meta->length(), std::move(buffers),
meta->null_count());
*out = MakeArray(arr_data);
return Status::OK();
}
diff --git a/cpp/src/arrow/python/numpy_to_arrow.cc
b/cpp/src/arrow/python/numpy_to_arrow.cc
index 0d2df9315..0c0d1a9b3 100644
--- a/cpp/src/arrow/python/numpy_to_arrow.cc
+++ b/cpp/src/arrow/python/numpy_to_arrow.cc
@@ -392,9 +392,7 @@ class NumPyConverter {
null_count = ValuesToBitmap<traits::npy_type>(arr_, null_bitmap_data_);
}
- BufferVector buffers = {null_bitmap_, data};
- auto arr_data =
- std::make_shared<ArrayData>(type_, length_, std::move(buffers),
null_count, 0);
+ auto arr_data = ArrayData::Make(type_, length_, {null_bitmap_, data},
null_count, 0);
return PushArray(arr_data);
}
@@ -472,8 +470,7 @@ Status CastBuffer(const std::shared_ptr<DataType>& in_type,
const std::shared_ptr<DataType>& out_type, MemoryPool* pool,
std::shared_ptr<Buffer>* out) {
// Must cast
- std::vector<std::shared_ptr<Buffer>> buffers = {valid_bitmap, input};
- auto tmp_data = std::make_shared<ArrayData>(in_type, length, buffers,
null_count);
+ auto tmp_data = ArrayData::Make(in_type, length, {valid_bitmap, input},
null_count);
std::shared_ptr<Array> tmp_array = MakeArray(tmp_data);
std::shared_ptr<Array> casted_array;
diff --git a/python/pyarrow/public-api.pxi b/python/pyarrow/public-api.pxi
index 9776f2ad7..2fdb606a7 100644
--- a/python/pyarrow/public-api.pxi
+++ b/python/pyarrow/public-api.pxi
@@ -44,7 +44,7 @@ cdef public api object pyarrow_wrap_buffer(const
shared_ptr[CBuffer]& buf):
cdef public api object pyarrow_wrap_resizable_buffer(
- const shared_ptr[CResizableBuffer]& buf):
+ const shared_ptr[CResizableBuffer]& buf):
cdef ResizableBuffer result = ResizableBuffer()
result.init_rz(buf)
return result
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> [C++] Add static ctor ArrayData::Make for nicer syntax in places
> ----------------------------------------------------------------
>
> Key: ARROW-1777
> URL: https://issues.apache.org/jira/browse/ARROW-1777
> Project: Apache Arrow
> Issue Type: New Feature
> Components: C++
> Reporter: Wes McKinney
> Assignee: Wes McKinney
> Labels: pull-request-available
> Fix For: 0.8.0
>
>
> Because of the fickleness of {{std::make_shared}}, we are having to store a
> vector of buffers in an lvalue rather than passing the buffers in the
> {{make_shared}} function call as an initializer list. This makes things a bit
> awkward, and could be made possibly better with a factory function
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
