bkietz commented on code in PR #37792:
URL: https://github.com/apache/arrow/pull/37792#discussion_r1334423433
##########
cpp/src/arrow/type_traits.h:
##########
@@ -801,8 +842,10 @@ using enable_if_has_c_type =
enable_if_t<has_c_type<T>::value, R>;
template <typename T>
using has_string_view =
std::integral_constant<bool, std::is_same<BinaryType, T>::value ||
+ std::is_same<BinaryViewType, T>::value ||
std::is_same<LargeBinaryType, T>::value ||
std::is_same<StringType, T>::value ||
+ std::is_same<StringViewType, T>::value ||
std::is_same<LargeStringType, T>::value ||
std::is_same<FixedSizeBinaryType,
T>::value>;
Review Comment:
I think it's acceptable to extend this. It's currently used in 5 places:
In HashTraits to decide if we need a BinaryMemoTable
https://github.com/apache/arrow/blob/868858f5d104a3e8e1cec8d7278ed88bb1f8bf3c/cpp/src/arrow/util/hashing.h#L898-L902
When unboxing scalars, types which satisfy this are unboxed to
std::string_view
https://github.com/apache/arrow/blob/868858f5d104a3e8e1cec8d7278ed88bb1f8bf3c/cpp/src/arrow/compute/kernels/codegen_internal.h#L347-L354
In the arrow-to-pandas conversion to detect whether we can memoize as
std::string_view
https://github.com/apache/arrow/blob/868858f5d104a3e8e1cec8d7278ed88bb1f8bf3c/python/pyarrow/src/arrow/python/arrow_to_pandas.cc#L594-L599
And in the python and r converters to decide if a dictionary is a stringy
type (should probably be a different predicate there)
https://github.com/apache/arrow/blob/868858f5d104a3e8e1cec8d7278ed88bb1f8bf3c/r/src/r_to_arrow.cpp#L963-L965
https://github.com/apache/arrow/blob/868858f5d104a3e8e1cec8d7278ed88bb1f8bf3c/python/pyarrow/src/arrow/python/python_to_arrow.cc#L769-L771
--
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]
