jorisvandenbossche commented on code in PR #14463:
URL: https://github.com/apache/arrow/pull/14463#discussion_r1163730869
##########
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;
+ }
+ std::shared_ptr<ArrayData> out_storage;
+ RETURN_NOT_OK(ConcatenateImpl(storage_data,
pool_).Concatenate(&out_storage));
Review Comment:
Yes, you can have an extension type that has a storage type that is an
extension type itself. And I suppose that in theory you could create two
extension types that point to each other as their storage type, creating an
infinite loop. But this is a pattern (i.e. calling the impl on the storage) we
use in other places as well, so you will already run into troubles anyway. I am
not sure how important it is we explicitly check for this everywhere.
--
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]
