jonkeane commented on code in PR #43351:
URL: https://github.com/apache/arrow/pull/43351#discussion_r1685621026
##########
r/src/arrow_cpp11.h:
##########
@@ -148,7 +148,7 @@ inline SEXP utf8_strings(SEXP x) {
for (R_xlen_t i = 0; i < n; i++, ++p_x) {
SEXP s = *p_x;
- if (s != NA_STRING) {
+ if (s != NA_STRING && ALTREP(s)) {
Review Comment:
This does not actually work yet. With this
https://github.com/apache/arrow/blob/c3ebdf500e75ca868f50b7d374fc8ce2237756b8/r/tests/testthat/test-utf.R#L19-L38
fail.
Before #43173 this line was:
```
if (s != NA_STRING && !IS_UTF8(s) && !IS_ASCII(s)) {
```
I suspect what's going on is that we caught vroom altrep vectors with this
if and called `SET_STRING_ELT` so we didn't get the `No Set_elt found for
ALTSTRING` error. But which `ALTREP(s)` we detect the non-utf strings here too
and attempt to `SET_STRING_ELT` when we shouldn't.
--
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]
