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: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org