jonkeane commented on code in PR #49712:
URL: https://github.com/apache/arrow/pull/49712#discussion_r4533040661
##########
r/src/r_to_arrow.cpp:
##########
@@ -863,54 +863,92 @@ class RPrimitiveConverter<T, enable_if_t<std::is_same<T,
FixedSizeBinaryType>::v
};
template <typename T>
-class RPrimitiveConverter<T, enable_if_string_like<T>>
+class RPrimitiveConverter<
+ T, enable_if_t<is_string_like_type<T>::value ||
is_string_view_type<T>::value>>
: public PrimitiveConverter<T, RConverter> {
public:
- using OffsetType = typename T::offset_type;
-
Status Extend(SEXP x, int64_t size, int64_t offset = 0) override {
RVectorType rtype = GetVectorType(x);
if (rtype != STRING) {
return Status::Invalid("Expecting a character vector");
}
- return UnsafeAppendUtf8Strings(arrow::r::utf8_strings(x), size, offset);
+ return AppendUtf8Strings(arrow::r::utf8_strings(x), size, offset);
}
void DelayedExtend(SEXP values, int64_t size, RTasks& tasks) override {
auto task = [this, values, size]() { return this->Extend(values, size); };
- // TODO: refine this., e.g. extract setup from Extend()
tasks.Append(false, std::move(task));
}
private:
- Status UnsafeAppendUtf8Strings(const cpp11::strings& s, int64_t size,
int64_t offset) {
- RETURN_NOT_OK(this->primitive_builder_->Reserve(s.size()));
+ Status AppendUtf8Strings(const cpp11::strings& s, int64_t size, int64_t
offset) {
Review Comment:
We change the name here from `Unsafe...` to `...` is that because this is no
longer unsafe? What did we change in here to do that? We still use
`UnsafeAppend` in the non-stringview branch. Maybe it's that this no longer is
_always_ unsafe, just sometimes (for data types most people use?). I won't die
on this hill, but it seems if it's important to flag safety in the name at all,
we would want it still here if this is sometimes unsafe.
--
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]
