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 aea76a11f45 GH-39688: [R] "Error: Filter expression not supported for
Arrow Datasets" using "date" expression rigth hand side of a filter (#51291)
aea76a11f45 is described below
commit aea76a11f457c8471cab59511e0833e73b8ac405
Author: Nic Crane <[email protected]>
AuthorDate: Wed Sep 23 13:35:16 2026 +0100
GH-39688: [R] "Error: Filter expression not supported for Arrow Datasets"
using "date" expression rigth hand side of a filter (#51291)
### Rationale for this change
Error when user tries to use local variable in dplyr pipeline in Arrow
### What changes are included in this PR?
Make sure we attach them to the mask
### Are these changes tested?
Yup
### Are there any user-facing changes?
Yep
* GitHub Issue: #39688
Authored-by: Nic Crane <[email protected]>
Signed-off-by: Nic Crane <[email protected]>
---
r/R/dplyr-eval.R | 28 ++++++++++++++++++++++++++++
r/tests/testthat/test-dplyr-filter.R | 22 ++++++++++++++++++++++
r/tests/testthat/test-dplyr-mutate.R | 15 +++++++++++++++
3 files changed, 65 insertions(+)
diff --git a/r/R/dplyr-eval.R b/r/R/dplyr-eval.R
index 5ed95c7646d..18fe1eb7c13 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 we bind it into the mask to match.
+ 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 a91f9719143..99d60d30b19 100644
--- a/r/tests/testthat/test-dplyr-filter.R
+++ b/r/tests/testthat/test-dplyr-filter.R
@@ -564,3 +564,25 @@ test_that("filter() and filter_out() with across() warn
about deprecation", {
class = "lifecycle_warning_deprecated"
)
})
+
+test_that("filter() with a variable from the calling environment", {
+ my_constant <- "d"
+ compare_dplyr_binding(
+ .input |>
+ filter(chr == my_constant) |>
+ collect(),
+ 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 7c4be302ffb..ae5b7d324b7 100644
--- a/r/tests/testthat/test-dplyr-mutate.R
+++ b/r/tests/testthat/test-dplyr-mutate.R
@@ -827,3 +827,18 @@ test_that("mutate() keeps the name given to if_any() and
if_all()", {
example_data
)
})
+
+test_that("mutate() with a variable that shares a name with a function
binding", {
+ # GH-39688: `abs` 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.
+ abs <- 1L
+ compare_dplyr_binding(
+ .input |>
+ mutate(
+ x = int + abs,
+ y = abs(int) + abs
+ ) |>
+ collect(),
+ tibble::tibble(int = -1:1)
+ )
+})
