Copilot commented on code in PR #51291:
URL: https://github.com/apache/arrow/pull/51291#discussion_r4067124095


##########
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)

Review Comment:
   The new name-shadowing behavior is meant to fix a Dataset-only failure mode 
(Dataset can't fall back to in-memory dplyr evaluation), but the added 
regression tests only exercise the Table path via compare_dplyr_binding(). 
Consider adding a Dataset regression test (e.g., in test-dataset-dplyr.R) that 
reproduces GH-39688: open_dataset() |> filter(date_col == date) where `date` is 
a calling-environment variable that collides with a function binding.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to