From d9f322e004caad03ff624e52d06ac719e1da6221 Mon Sep 17 00:00:00 2001 From: Sumit Chauhan Date: Mon, 7 Sep 2026 11:24:20 +0530 Subject: [PATCH] GH-33432: [R] Match base/stringr semantics for str_replace() with NA replacement base::sub()/gsub() and stringr::str_replace()/str_replace_all() set the whole string to NA when the replacement is NA and the pattern matches. The Acero replace_substring[_regex] kernels instead splice a literal "NA" into the string. Special-case an NA replacement in the binding and rewrite it as if_else(, NA, x). Co-Authored-By: Claude Sonnet 5 --- r/R/dplyr-funcs-string.R | 20 ++++++++++++++++ r/tests/testthat/test-dplyr-funcs-string.R | 27 ++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/r/R/dplyr-funcs-string.R b/r/R/dplyr-funcs-string.R index 158bae2db87c..4158932aa367 100644 --- a/r/R/dplyr-funcs-string.R +++ b/r/R/dplyr-funcs-string.R @@ -367,6 +367,26 @@ 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(, NA, x). GH-33432 + if (is.na(replacement)) { + is_match <- Expression$create( + ifelse(fixed && !ignore.case, "match_substring", "match_substring_regex"), + x, + options = list( + pattern = format_string_pattern(pattern, ignore.case, fixed), + ignore_case = FALSE + ) + ) + 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 58da3ea23358..3b7488d74bee 100644 --- a/r/tests/testthat/test-dplyr-funcs-string.R +++ b/r/tests/testthat/test-dplyr-funcs-string.R @@ -428,6 +428,33 @@ 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_) + ) |> + 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")