felipecrv commented on code in PR #14463:
URL: https://github.com/apache/arrow/pull/14463#discussion_r1163168868
##########
cpp/src/arrow/array/concatenate.cc:
##########
@@ -459,8 +459,17 @@ class ConcatenateImpl {
}
Status Visit(const ExtensionType& e) {
- // XXX can we just concatenate their storage?
- return Status::NotImplemented("concatenation of ", e);
+ ArrayDataVector storage_data(in_.size());
+ for (size_t i = 0; i < in_.size(); ++i) {
+ auto storage = in_[i]->Copy();
+ storage->type = e.storage_type();
+ storage_data[i] = storage;
Review Comment:
You can `std::move(storage)` here to avoid unnecessary increments of
ref-counts on the shared_ptr.
The whole loop body can be free of unnecessary code if re-written as:
```cpp
storage_data[i] = in_[i]->Copy();
storage_data[i]->type = e.storage_type();
```
And another way.
```cpp
ArrayDataVector storage_data;
storage_data.reserve(in_.size());
for (auto &input : in_) {
storage_data.emplace_back(input->Copy())->type = e.storage_type();
}
```
The last two avoid having to check the refcount of `storage` when its dtor
is called at the end of the body.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
