pitrou commented on code in PR #13783:
URL: https://github.com/apache/arrow/pull/13783#discussion_r936591436
##########
cpp/src/arrow/type.h:
##########
@@ -2151,4 +2151,32 @@ const std::vector<std::shared_ptr<DataType>>&
IntervalTypes();
ARROW_EXPORT
const std::vector<std::shared_ptr<DataType>>& PrimitiveTypes();
+ARROW_EXPORT
+bool IsSignedIntType(std::shared_ptr<DataType>);
Review Comment:
I'm leaning towards -1 on adding these APIs. There are similar APIs, with
different names and inlined, that take a type id. It's not significantly to
write `IsSignedIntType(type)` rather than `is_signed_integer(type->id())`.
##########
cpp/src/arrow/type.h:
##########
@@ -2151,4 +2151,32 @@ const std::vector<std::shared_ptr<DataType>>&
IntervalTypes();
ARROW_EXPORT
const std::vector<std::shared_ptr<DataType>>& PrimitiveTypes();
+ARROW_EXPORT
+bool IsSignedIntType(std::shared_ptr<DataType>);
Review Comment:
In addition, the signature is inefficient as this is always copying a
shared_ptr (implying an atomic reference increment). Instead this could just
take a `const DataType&`.
##########
cpp/src/arrow/type.cc:
##########
@@ -2494,6 +2519,61 @@ const std::vector<std::shared_ptr<DataType>>&
PrimitiveTypes() {
return g_primitive_types;
}
+bool IsBaseBinaryType(std::shared_ptr<DataType> type) {
+ std::call_once(static_data_initialized, InitStaticData);
+ return g_base_binary_types_set.find(type) != g_base_binary_types_set.end();
Review Comment:
This is really an inefficient way to do it instead of simply switching based
on the `id()`.
--
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]
