rok commented on code in PR #13487:
URL: https://github.com/apache/arrow/pull/13487#discussion_r964221860
##########
cpp/src/arrow/compute/light_array.cc:
##########
@@ -107,42 +107,96 @@ KeyColumnArray KeyColumnArray::Slice(int64_t offset,
int64_t length) const {
return sliced;
}
-Result<KeyColumnMetadata> ColumnMetadataFromDataType(
- const std::shared_ptr<DataType>& type) {
- const bool is_extension = type->id() == Type::EXTENSION;
- const std::shared_ptr<DataType>& typ =
- is_extension
- ?
arrow::internal::checked_pointer_cast<ExtensionType>(type->GetSharedPtr())
- ->storage_type()
- : type;
-
- if (typ->id() == Type::DICTIONARY) {
+Result<KeyColumnMetadata> ColumnMetadataFromDataType(const DataType* type) {
+ // "ptype" is the "physical" type
+ const DataType* ptype = type;
+
+ // For ExtensionType, use the backing physical type (storage_type() is a
shared ptr)
+ if (ARROW_PREDICT_FALSE(type->id() == Type::EXTENSION)) {
+ const ExtensionType* ext_type = static_cast<const ExtensionType*>(type);
+ ptype = ext_type->storage_type().get();
+ }
Review Comment:
This now potentially defines `ptype` twice, but I don't think it's an issue
and looks way nicer.
--
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]
