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



##########
File path: r/src/altrep.cpp
##########
@@ -44,25 +49,58 @@ extern "C" {
 #endif
 
 #include <arrow/array.h>
+#include <arrow/util/bitmap_reader.h>
+
+#include "./r_task_group.h"
 
 namespace arrow {
 namespace r {
 
+template <typename T>
+void UseSentinel(const std::shared_ptr<Array>& array) {

Review comment:
       Chances are we might need to implement something like this for arrays 
that are backed by an immutable buffer, so maybe I can do that, and then we can 
compare/measure and evaluate how risky this is. 
   
   Then we have 3 cases: 
    - Array with no nulls: best altrep with DATAPTR(), no need for setting 
sentinels (what we had prior to this pull request). 
    - Array with some nulls, immutable: using GetRegion, I guess with a 
materialized R vector in the data2 altrep slot, so that DATAPTR() materializes 
and returns this. 
    
    - Array with some nulls, "mutable": hacky proposal of this pr. 
   
   Perhaps we only need the first 2. 
   
   
   
   
   
   




-- 
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]


Reply via email to