Skip to content

Navigation Menu

Sign in
Sign up

GH-39688: [R] "Error: Filter expression not supported for Arrow Datasets" using "date" expression rigth hand side of a filter #51291

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
thisisnic wants to merge 1 commit into apache:main
base: main
Choose a base branch
Loading
from thisisnic:GH-39688-shadowed-names
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions r/R/dplyr-eval.R
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)) {
Comment on lines +147 to +151
mask[[var_name]] <- user_var
}
}
}
invisible()
}

get_standard_error_messages <- function() {
if (is.null(.cache$i18ized_error_pattern)) {
# Memoize it
Expand Down
12 changes: 12 additions & 0 deletions r/tests/testthat/test-dplyr-filter.R
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
})
15 changes: 15 additions & 0 deletions r/tests/testthat/test-dplyr-mutate.R
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
})
Loading

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