Skip to content

Conversation

@HyukjinKwon
Copy link
Member

@HyukjinKwon HyukjinKwon commented Dec 26, 2025

Rationale for this change

03f8ae7 added a couple of TODOs for testing aggregation expressions

# TODO: add a test for this

# TODO: add a test for this

Should test negative cases too as mentioned in the TODOs.

What changes are included in this PR?

This PP adds a couple of tests for filter() and arrange() with aggregation expressions

Are these changes tested?

Unittests added.

Are there any user-facing changes?

No, test-only.

@github-actions
Copy link

⚠️ GitHub issue #48660 has been automatically assigned in GitHub to PR creator.


test_that("arrange() with aggregation expressions errors", {
tab <- arrow_table(tbl)
expect_warning(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has to catch warning instead per:

── Warning ('test-dplyr-filter.R:496:3'): filter() with aggregation expressions errors ──
In int < mean(int): 
i Expression not supported in filter() in Arrow
> Pulling data into R
Backtrace:
     ▆
  1. ├─testthat::expect_error(filter(tab, int < mean(int)), "not supported in filter") at test-dplyr-filter.R:496:3
  2. │ └─testthat:::expect_condition_matching_(...)
  3. │   └─testthat:::quasi_capture(...)
  4. │     ├─testthat (local) .capture(...)
  5. │     │ └─base::withCallingHandlers(...)
  6. │     └─rlang::eval_bare(quo_get_expr(.quo), quo_get_env(.quo))
  7. ├─dplyr::filter(tab, int < mean(int))
  8. └─arrow:::filter.ArrowTabular(tab, int < mean(int))
  9.   └─arrow:::try_arrow_dplyr(...)
 10.     └─base::tryCatch(...)
 11.       └─base (local) tryCatchList(expr, classes, parentenv, handlers)
 12.         └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
 13.           └─value[[3L]](cond)
 14.             └─arrow:::abandon_ship(e, parent)

@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Dec 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant