dmitry-chirkov-dremio commented on code in PR #51647: URL: https://github.com/apache/arrow/pull/51647#discussion_r4369392943
########## cpp/src/arrow/util/list_util.cc: ########## @@ -203,10 +204,16 @@ Result<std::shared_ptr<ArrayData>> ListFromListViewImpl(const ArraySpan& list_vi const auto& value_type = list_view_type.value_type(); // The value type is carried over, so that callers may cast the child. auto list_type = std::make_shared<DestListType>(value_type); + // MakeBuilder does not support extension types, so gather the values using + // the storage type and restore the extension type afterwards. + const bool is_extension = value_type->id() == Type::EXTENSION; + const auto builder_value_type = + is_extension ? checked_cast<const ExtensionType&>(*value_type).storage_type() + : value_type; ARROW_ASSIGN_OR_RAISE(auto sum_of_sizes, SumOfLogicalListSizes(list_view)); ARROW_ASSIGN_OR_RAISE(std::shared_ptr<ArrayBuilder> value_builder, - MakeBuilder(value_type, pool)); + MakeBuilder(builder_value_type, pool)); Review Comment: We fixed the direct extension child, but put the same type inside a struct or another list and it fails again. These casts pass on the parent. This makes me doubt that the top-level check is enough. Before adding another special case, please check how gathering should handle nested extensions and add tests for them. I would keep changes requested for now. -- 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]
