[
https://issues.apache.org/jira/browse/ARROW-1671?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16214515#comment-16214515
]
ASF GitHub Bot commented on ARROW-1671:
---------------------------------------
wesm closed pull request #1227: ARROW-1671: [C++] Deprecate arrow::MakeArray
that returns Status, refactor existing code to new variant
URL: https://github.com/apache/arrow/pull/1227
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 a7930a139..eaac187a3 100644
--- a/cpp/src/arrow/array.cc
+++ b/cpp/src/arrow/array.cc
@@ -97,9 +97,7 @@ static inline std::shared_ptr<ArrayData> SliceData(const
ArrayData& data, int64_
}
std::shared_ptr<Array> Array::Slice(int64_t offset, int64_t length) const {
- std::shared_ptr<Array> result;
- DCHECK(MakeArray(SliceData(*data_, offset, length), &result).ok());
- return result;
+ return MakeArray(SliceData(*data_, offset, length));
}
std::shared_ptr<Array> Array::Slice(int64_t offset) const {
@@ -210,7 +208,7 @@ void ListArray::SetData(const std::shared_ptr<ArrayData>&
data) {
raw_value_offsets_ = value_offsets == nullptr
? nullptr
: reinterpret_cast<const
int32_t*>(value_offsets->data());
- DCHECK(MakeArray(data_->child_data[0], &values_).ok());
+ values_ = MakeArray(data_->child_data[0]);
}
std::shared_ptr<DataType> ListArray::value_type() const {
@@ -323,7 +321,7 @@ StructArray::StructArray(const std::shared_ptr<DataType>&
type, int64_t length,
std::shared_ptr<Array> StructArray::field(int i) const {
if (!boxed_fields_[i]) {
- DCHECK(MakeArray(data_->child_data[i], &boxed_fields_[i]).ok());
+ boxed_fields_[i] = MakeArray(data_->child_data[i]);
}
DCHECK(boxed_fields_[i]);
return boxed_fields_[i];
@@ -369,7 +367,7 @@ UnionArray::UnionArray(const std::shared_ptr<DataType>&
type, int64_t length,
std::shared_ptr<Array> UnionArray::child(int i) const {
if (!boxed_fields_[i]) {
- DCHECK(MakeArray(data_->child_data[i], &boxed_fields_[i]).ok());
+ boxed_fields_[i] = MakeArray(data_->child_data[i]);
}
DCHECK(boxed_fields_[i]);
return boxed_fields_[i];
@@ -377,7 +375,7 @@ std::shared_ptr<Array> UnionArray::child(int i) const {
const Array* UnionArray::UnsafeChild(int i) const {
if (!boxed_fields_[i]) {
- DCHECK(MakeArray(data_->child_data[i], &boxed_fields_[i]).ok());
+ boxed_fields_[i] = MakeArray(data_->child_data[i]);
}
DCHECK(boxed_fields_[i]);
return boxed_fields_[i].get();
@@ -407,7 +405,7 @@ void DictionaryArray::SetData(const
std::shared_ptr<ArrayData>& data) {
auto indices_data = data_->ShallowCopy();
indices_data->type = dict_type_->index_type();
std::shared_ptr<Array> result;
- DCHECK(MakeArray(indices_data, &indices_).ok());
+ indices_ = MakeArray(indices_data);
}
std::shared_ptr<Array> DictionaryArray::indices() const { return indices_; }
@@ -589,6 +587,8 @@ class ArrayDataWrapper {
} // namespace internal
+#ifndef ARROW_NO_DEPRECATED_API
+
Status MakeArray(const std::shared_ptr<ArrayData>& data,
std::shared_ptr<Array>* out) {
internal::ArrayDataWrapper wrapper_visitor(data, out);
RETURN_NOT_OK(VisitTypeInline(*data->type, &wrapper_visitor));
@@ -596,6 +596,17 @@ Status MakeArray(const std::shared_ptr<ArrayData>& data,
std::shared_ptr<Array>*
return Status::OK();
}
+#endif
+
+std::shared_ptr<Array> MakeArray(const std::shared_ptr<ArrayData>& data) {
+ std::shared_ptr<Array> out;
+ internal::ArrayDataWrapper wrapper_visitor(data, &out);
+ Status s = VisitTypeInline(*data->type, &wrapper_visitor);
+ DCHECK(s.ok());
+ DCHECK(out);
+ return out;
+}
+
// ----------------------------------------------------------------------
// Instantiate templates
diff --git a/cpp/src/arrow/array.h b/cpp/src/arrow/array.h
index 0805cad3d..75dda4a75 100644
--- a/cpp/src/arrow/array.h
+++ b/cpp/src/arrow/array.h
@@ -144,13 +144,25 @@ struct ARROW_EXPORT ArrayData {
std::vector<std::shared_ptr<ArrayData>> child_data;
};
+#ifndef ARROW_NO_DEPRECATED_API
+
/// \brief Create a strongly-typed Array instance from generic ArrayData
/// \param[in] data the array contents
/// \param[out] out the resulting Array instance
/// \return Status
+///
+/// \note Deprecated since 0.8.0
ARROW_EXPORT
Status MakeArray(const std::shared_ptr<ArrayData>& data,
std::shared_ptr<Array>* out);
+#endif
+
+/// \brief Create a strongly-typed Array instance from generic ArrayData
+/// \param[in] data the array contents
+/// \return the resulting Array instance
+ARROW_EXPORT
+std::shared_ptr<Array> MakeArray(const std::shared_ptr<ArrayData>& data);
+
// ----------------------------------------------------------------------
// User array accessor types
diff --git a/cpp/src/arrow/builder.cc b/cpp/src/arrow/builder.cc
index 331de2d36..c910170dd 100644
--- a/cpp/src/arrow/builder.cc
+++ b/cpp/src/arrow/builder.cc
@@ -102,7 +102,8 @@ Status ArrayBuilder::Advance(int64_t elements) {
Status ArrayBuilder::Finish(std::shared_ptr<Array>* out) {
std::shared_ptr<ArrayData> internal_data;
RETURN_NOT_OK(FinishInternal(&internal_data));
- return MakeArray(internal_data, out);
+ *out = MakeArray(internal_data);
+ return Status::OK();
}
Status ArrayBuilder::Reserve(int64_t elements) {
diff --git a/cpp/src/arrow/compute/cast.cc b/cpp/src/arrow/compute/cast.cc
index 2381e1ea3..e8bbfd347 100644
--- a/cpp/src/arrow/compute/cast.cc
+++ b/cpp/src/arrow/compute/cast.cc
@@ -717,7 +717,8 @@ Status Cast(FunctionContext* ctx, const Array& array,
auto out_data = std::make_shared<ArrayData>(out_type, array.length());
RETURN_NOT_OK(func->Call(ctx, array, out_data.get()));
- return MakeArray(out_data, out);
+ *out = MakeArray(out_data);
+ return Status::OK();
}
} // namespace compute
diff --git a/cpp/src/arrow/compute/compute-test.cc
b/cpp/src/arrow/compute/compute-test.cc
index 602acff20..8a595178d 100644
--- a/cpp/src/arrow/compute/compute-test.cc
+++ b/cpp/src/arrow/compute/compute-test.cc
@@ -369,8 +369,8 @@ TEST_F(TestCast, PreallocatedMemory) {
// Buffer address unchanged
ASSERT_EQ(out_values.get(), out_data->buffers[1].get());
- std::shared_ptr<Array> result, expected;
- ASSERT_OK(MakeArray(out_data, &result));
+ std::shared_ptr<Array> result = MakeArray(out_data);
+ std::shared_ptr<Array> expected;
ArrayFromVector<Int64Type, int64_t>(int64(), is_valid, e1, &expected);
AssertArraysEqual(*expected, *result);
diff --git a/cpp/src/arrow/ipc/feather.cc b/cpp/src/arrow/ipc/feather.cc
index 97ed601ad..cea720bd0 100644
--- a/cpp/src/arrow/ipc/feather.cc
+++ b/cpp/src/arrow/ipc/feather.cc
@@ -371,7 +371,8 @@ class TableReader::TableReaderImpl {
auto arr_data =
std::make_shared<ArrayData>(type, meta->length(), buffers,
meta->null_count());
- return MakeArray(arr_data, out);
+ *out = MakeArray(arr_data);
+ return Status::OK();
}
bool HasDescription() const { return metadata_->HasDescription(); }
@@ -490,7 +491,8 @@ static Status SanitizeUnsupportedTypes(const Array& values,
std::shared_ptr<Arra
values.null_bitmap(),
values.null_count());
return Status::OK();
} else {
- return MakeArray(values.data(), out);
+ *out = MakeArray(values.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 27ee2302e..2c89a9f61 100644
--- a/cpp/src/arrow/python/numpy_to_arrow.cc
+++ b/cpp/src/arrow/python/numpy_to_arrow.cc
@@ -364,9 +364,7 @@ class NumPyConverter {
}
Status PushArray(const std::shared_ptr<ArrayData>& data) {
- std::shared_ptr<Array> result;
- RETURN_NOT_OK(MakeArray(data, &result));
- out_arrays_.emplace_back(std::move(result));
+ out_arrays_.emplace_back(MakeArray(data));
return Status::OK();
}
@@ -495,8 +493,8 @@ static Status CastBuffer(const std::shared_ptr<Buffer>&
input, const int64_t len
std::vector<std::shared_ptr<Buffer>> buffers = {nullptr, input};
auto tmp_data = std::make_shared<ArrayData>(in_type, length, buffers, 0);
- std::shared_ptr<Array> tmp_array, casted_array;
- RETURN_NOT_OK(MakeArray(tmp_data, &tmp_array));
+ std::shared_ptr<Array> tmp_array = MakeArray(tmp_data);
+ std::shared_ptr<Array> casted_array;
compute::FunctionContext context(pool);
compute::CastOptions cast_options;
diff --git a/cpp/src/arrow/table.cc b/cpp/src/arrow/table.cc
index aac5dad1f..fe19bf4ce 100644
--- a/cpp/src/arrow/table.cc
+++ b/cpp/src/arrow/table.cc
@@ -192,7 +192,7 @@ RecordBatch::RecordBatch(const std::shared_ptr<Schema>&
schema, int64_t num_rows
std::shared_ptr<Array> RecordBatch::column(int i) const {
if (!boxed_columns_[i]) {
- DCHECK(MakeArray(columns_[i], &boxed_columns_[i]).ok());
+ boxed_columns_[i] = MakeArray(columns_[i]);
}
DCHECK(boxed_columns_[i]);
return boxed_columns_[i];
----------------------------------------------------------------
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:
us...@infra.apache.org
> [C++] Change arrow::MakeArray to not return Status
> --------------------------------------------------
>
> Key: ARROW-1671
> URL: https://issues.apache.org/jira/browse/ARROW-1671
> Project: Apache Arrow
> Issue Type: Improvement
> Components: C++
> Reporter: Wes McKinney
> Assignee: Wes McKinney
> Labels: pull-request-available
> Fix For: 0.8.0
>
>
> It should not be possible for this function to fail. We can do a DCHECK
> internally of any internal Status values for testing purposes
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
