This is an automated email from the ASF dual-hosted git repository.
thisisnic pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 0241012442 GH-33432: [R] Match base/stringr semantics for
str_replace() with NA replacement (#51197)
0241012442 is described below
commit 0241012442c00da87b777e24a37e1d155e80e806
Author: Sumit Chauhan <[email protected]>
AuthorDate: Sat Sep 12 16:48:53 2026 +0530
GH-33432: [R] Match base/stringr semantics for str_replace() with NA
replacement (#51197)
### Rationale for this change
`base::sub()`/`gsub()` and `stringr::str_replace()`/`str_replace_all()` set
the whole
element to `NA` when the replacement is `NA` and the pattern matches. The
Acero
`replace_substring[_regex]` kernels don't support that and splice a literal
`"NA"`
into the string, e.g. `str_replace("one", "o", NA_character_)` returns
`"NAne"`
instead of `NA`.
### What changes are included in this PR?
Special-case an `NA` `replacement` in the shared
`sub`/`gsub`/`str_replace`/`str_replace_all`
binding helper and rewrite it as `if_else(<pattern matches>, NA, x)` using
`match_substring[_regex]`. Covers regex, fixed, and ignore-case patterns.
### Are these changes tested?
Yes — new test in `test-dplyr-funcs-string.R` comparing against
base/stringr for
regex/fixed/ignore-case, empty strings, and `NA` input.
### Are there any user-facing changes?
Bug fix only; `str_replace(x, p, NA)` now returns `NA` on match instead of a
corrupted string.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
* GitHub Issue: #33432
Lead-authored-by: Sumit Chauhan <[email protected]>
Co-authored-by: Nic Crane <[email protected]>
Signed-off-by: Nic Crane <[email protected]>
---
r/R/dplyr-funcs-string.R | 18 ++++++++++++++++++
r/tests/testthat/test-dplyr-funcs-string.R | 28 ++++++++++++++++++++++++++++
2 files changed, 46 insertions(+)
diff --git a/r/R/dplyr-funcs-string.R b/r/R/dplyr-funcs-string.R
index 158bae2db8..da2f729e28 100644
--- a/r/R/dplyr-funcs-string.R
+++ b/r/R/dplyr-funcs-string.R
@@ -367,6 +367,24 @@ register_bindings_string_regex <- function() {
if (length(replacement) != 1) {
validation_error("`replacement` must be a length 1 character vector")
}
+ # An NA replacement sets the whole string to NA wherever the pattern
+ # matches, matching base::sub()/gsub() and stringr::str_replace(). The
+ # replace_substring[_regex] kernels don't support this, so rewrite it as
+ # if_else(<pattern matches>, NA, x). GH-33432
+ if (is.na(replacement)) {
+ is_match <- create_string_match_expr(
+ ifelse(fixed, "match_substring", "match_substring_regex"),
+ string = x,
+ pattern = pattern,
+ ignore_case = ignore.case
+ )
+ return(Expression$create(
+ "if_else",
+ is_match,
+ Expression$scalar(NA_character_),
+ x
+ ))
+ }
Expression$create(
ifelse(fixed && !ignore.case, "replace_substring",
"replace_substring_regex"),
x,
diff --git a/r/tests/testthat/test-dplyr-funcs-string.R
b/r/tests/testthat/test-dplyr-funcs-string.R
index 58da3ea233..59ae85afba 100644
--- a/r/tests/testthat/test-dplyr-funcs-string.R
+++ b/r/tests/testthat/test-dplyr-funcs-string.R
@@ -428,6 +428,34 @@ test_that("sub and gsub with namespacing", {
)
})
+test_that("str_replace/sub with an NA replacement match base/stringr
(GH-33432)", {
+ df <- tibble(x = c("", "one", "two", "three", "four", NA))
+
+ # A match sets the whole value to NA; non-matches and NA input are unchanged
+ compare_dplyr_binding(
+ .input |>
+ transmute(
+ regex = str_replace(x, "o", NA_character_),
+ regex_all = str_replace_all(x, "o", NA_character_),
+ fixed = str_replace_all(x, fixed("o"), NA_character_),
+ ci = str_replace_all(x, regex("O", ignore_case = TRUE), NA_character_),
+ fixed_ci = str_replace_all(x, fixed("O", ignore_case = TRUE),
NA_character_)
+ ) |>
+ collect(),
+ df
+ )
+
+ compare_dplyr_binding(
+ .input |>
+ transmute(
+ subbed = sub("o", NA_character_, x),
+ gsubbed = gsub("o", NA_character_, x)
+ ) |>
+ collect(),
+ df
+ )
+})
+
test_that("str_replace and str_replace_all", {
x <- Expression$field_ref("x")
