From 7a010506e80ac207ef2659dde0f65db3a3f6cc7d Mon Sep 17 00:00:00 2001 From: Nic Crane Date: 2026年9月10日 18:04:14 +0100 Subject: [PATCH] Look in mask for user variables --- r/R/dplyr-eval.R | 28 ++++++++++++++++++++++++++++ r/tests/testthat/test-dplyr-filter.R | 12 ++++++++++++ r/tests/testthat/test-dplyr-mutate.R | 15 +++++++++++++++ 3 files changed, 55 insertions(+) diff --git a/r/R/dplyr-eval.R b/r/R/dplyr-eval.R index 1282f171878d..8322346f3eb9 100644 --- a/r/R/dplyr-eval.R +++ b/r/R/dplyr-eval.R @@ -23,6 +23,10 @@ arrow_eval <- function(expr, mask) { # but it wouldn't have worked anyway!) # Note this is *not* true UDFs. add_user_functions_to_mask(expr, mask) + # Likewise, look for R variables referenced in expr that share a name with a + # function binding (like `date` or `day`) and add them to the mask, so that + # the user's variable is found rather than the binding, as dplyr would do. + add_user_variables_to_mask(expr, mask) # This yields an Expression as long as the `exprs` are implemented in Arrow. # Otherwise, it raises a classed error, either: @@ -128,6 +132,30 @@ add_user_functions_to_mask <- function(expr, mask) { invisible() } +add_user_variables_to_mask <- function(expr, mask) { + # The function bindings environment sits between the columns and the user's + # environment in the mask, so a symbol like `date` in `filter(Date == date)` + # would resolve to the `date()` binding rather than the user's variable. + # dplyr would find the variable, so bind it into the mask so we do too. + if (is_quosure(expr)) { + function_env <- parent.env(parent.env(mask)) + quo_env <- quo_get_env(expr) + # all.vars() returns symbols that aren't in function position + vars_in_expr <- all.vars(quo_get_expr(expr)) + columns <- names(mask$.data) + shadowed <- setdiff(intersect(vars_in_expr, ls(function_env, all.names = TRUE)), columns) + for (var_name in shadowed) { + user_var <- get0(var_name, quo_env) + # Functions from the user's environment (like lubridate::day) shouldn't + # take precedence over the bindings + if (!is.null(user_var) && !is.function(user_var)) { + mask[[var_name]] <- user_var + } + } + } + invisible() +} + get_standard_error_messages <- function() { if (is.null(.cache$i18ized_error_pattern)) { # Memoize it diff --git a/r/tests/testthat/test-dplyr-filter.R b/r/tests/testthat/test-dplyr-filter.R index ad69b26be798..e9bc1595e7c6 100644 --- a/r/tests/testthat/test-dplyr-filter.R +++ b/r/tests/testthat/test-dplyr-filter.R @@ -547,3 +547,15 @@ test_that("More complex select/filter_out", { tbl ) }) + +test_that("filter() with a variable that shares a name with a function binding", { + # GH-39688: `date` and `day` are also function bindings + date <- "d" + day <- 5L + compare_dplyr_binding( + .input |> + filter(chr == date, int <= day) |> + collect(), + tbl + ) +}) diff --git a/r/tests/testthat/test-dplyr-mutate.R b/r/tests/testthat/test-dplyr-mutate.R index 63f69227b289..30bb16e41108 100644 --- a/r/tests/testthat/test-dplyr-mutate.R +++ b/r/tests/testthat/test-dplyr-mutate.R @@ -775,3 +775,18 @@ test_that("across() does not select grouping variables within transmute()", { "Column `chr` doesn't exist" ) }) + +test_that("mutate() with a variable that shares a name with a function binding", { + # GH-39688: `day` is also a function binding. The user's variable should be + # found when used as a value, and the binding when used as a function. + day <- 1L + compare_dplyr_binding( + .input |> + mutate( + x = int + day, + y = lubridate::day(date) + day + ) |> + collect(), + tibble::tibble(int = 1:3, date = as.Date("2024-01-18") + 0:2) + ) +})

AltStyle によって変換されたページ (->オリジナル) /