amoeba commented on code in PR #45951:
URL: https://github.com/apache/arrow/pull/45951#discussion_r2020338733
##########
r/src/altrep.cpp:
##########
@@ -1267,24 +1304,27 @@ sexp test_arrow_altrep_copy_by_dataptr(sexp x) {
if (TYPEOF(x) == INTSXP) {
cpp11::writable::integers out(Rf_xlength(x));
- int* ptr = reinterpret_cast<int*>(DATAPTR(x));
+ int* ptr = reinterpret_cast<int*>(INTEGER(x));
for (R_xlen_t i = 0; i < n; i++) {
out[i] = ptr[i];
}
return out;
} else if (TYPEOF(x) == REALSXP) {
cpp11::writable::doubles out(Rf_xlength(x));
- double* ptr = reinterpret_cast<double*>(DATAPTR(x));
+ double* ptr = reinterpret_cast<double*>(REAL(x));
for (R_xlen_t i = 0; i < n; i++) {
out[i] = ptr[i];
}
return out;
} else if (TYPEOF(x) == STRSXP) {
cpp11::writable::strings out(Rf_xlength(x));
- SEXP* ptr = reinterpret_cast<SEXP*>(DATAPTR(x));
- for (R_xlen_t i = 0; i < n; i++) {
- out[i] = ptr[i];
+ R_xlen_t len = Rf_xlength(x);
+
+ for (R_xlen_t i = 0; i < len; i++) {
+ SEXP str_elt = reinterpret_cast<SEXP>(STRING_ELT(x, i));
+ out[i] = str_elt;
Review Comment:
`DATAPTR` and `STRING_ELT` work a bit different for altrep so it seems
reasonable to me to assume we were implicitly materializing before and no
longer are with `STRING_ELT`. It seems like this is an assumption of the test
that no longer holds and the expectation could be removed.
--
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
