romainfrancois commented on a change in pull request #11369:
URL: https://github.com/apache/arrow/pull/11369#discussion_r725146485



##########
File path: r/src/altrep.cpp
##########
@@ -215,9 +219,10 @@ struct AltrepVectorPrimitive : public 
AltrepVectorBase<AltrepVectorPrimitive<sex
 
   // The value at position i
   static c_type Elt(SEXP alt, R_xlen_t i) {
-    const auto& array = Base::GetArray(alt);
-    return array->IsNull(i) ? cpp11::na<c_type>()
-                            : array->data()->template GetValues<c_type>(1)[i];
+    auto array = Base::GetChunkedArray(alt)->Slice(i, 1)->chunk(0);

Review comment:
       This should use scalar instead of making a size 1 slice. 

##########
File path: r/src/altrep.cpp
##########
@@ -436,15 +450,16 @@ struct AltrepVectorString : public 
AltrepVectorBase<AltrepVectorString<Type>> {
 
     BEGIN_CPP11
 
-    const auto& array = Base::GetArray(alt);
-    RStringViewer r_string_viewer(array);
+    auto array = Base::GetChunkedArray(alt)->Slice(i, 1)->chunk(0);

Review comment:
       Maybe this could use a scalar and `RStringViewer` would deal with it ?




-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to