jonkeane commented on code in PR #41223:
URL: https://github.com/apache/arrow/pull/41223#discussion_r1567299608
##########
r/tests/testthat/test-dplyr-summarize.R:
##########
@@ -1083,11 +1121,6 @@ test_that("summarise() can handle scalars and literal
values", {
tibble(y = 1L)
)
- expect_identical(
- record_batch(tbl) %>% summarise(y = Expression$scalar(1L)) %>% collect(),
- tibble(y = 1L)
- )
Review Comment:
Absolutely agree there. We can add this as follow on, but my concern is if
someone has become convinced that `Expression$scalar()` is needed there, if the
error is one of our more generic ones or isn't clear they won't know enough to
get out of that.
##########
r/R/dplyr-eval.R:
##########
@@ -20,6 +20,8 @@ arrow_eval <- function(expr, mask) {
# with references to Arrays (if .data is Table/RecordBatch) or Fields (if
# .data is a Dataset).
+ add_user_functions_to_mask(expr, mask)
Review Comment:
That's fantastic, thanks
--
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]
