diff --git a/NEWS.md b/NEWS.md index 0f1748db9..dae72d35d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,8 @@ # dbplyr (development version) +* `across(everything())` doesn't select grouping columns created via `.by` in + `summarise()` (@mgirlich, #1493). + # dbplyr 2.5.0 ## Improved tools for qualified table names diff --git a/R/verb-summarise.R b/R/verb-summarise.R index 9e4ad020e..8c7572f82 100644 --- a/R/verb-summarise.R +++ b/R/verb-summarise.R @@ -35,7 +35,6 @@ #' show_query() summarise.tbl_lazy <- function(.data, ..., .by = NULL, .groups = NULL) { check_groups(.groups) - dots <- summarise_eval_dots(.data, ...) by <- compute_by( {{ .by }}, @@ -49,6 +48,8 @@ summarise.tbl_lazy <- function(.data, ..., .by = NULL, .groups = NULL) { .data$lazy_query$group_vars <- by$names .groups <- "drop" } + + dots <- summarise_eval_dots(.data, ...) .data$lazy_query <- add_summarise( .data, dots, .groups = .groups, diff --git a/tests/testthat/_snaps/tbl-sql.md b/tests/testthat/_snaps/tbl-sql.md index ada327233..f207e90ea 100644 --- a/tests/testthat/_snaps/tbl-sql.md +++ b/tests/testthat/_snaps/tbl-sql.md @@ -28,12 +28,8 @@ # check_from is deprecated Code - tbl(con, "x", check_from = FALSE) + out <- tbl(con, "x", check_from = FALSE) Condition Warning: The `check_from` argument of `tbl_sql()` is deprecated as of dbplyr 2.5.0. - Output - # Source: table<`x`> [0 x 1] - # Database: sqlite 3.45.0 [:memory:] - # i 1 variable: y diff --git a/tests/testthat/_snaps/verb-summarise.md b/tests/testthat/_snaps/verb-summarise.md index 270c237c8..476125d87 100644 --- a/tests/testthat/_snaps/verb-summarise.md +++ b/tests/testthat/_snaps/verb-summarise.md @@ -102,6 +102,16 @@ FROM `df` GROUP BY `g` +# across doesn't select columns from `.by` #1493 + + Code + out + Output + + SELECT `g`, SUM(`..x`) AS `x` + FROM `df` + GROUP BY `g` + # can't use `.by` with `.groups` Code diff --git a/tests/testthat/test-tbl-sql.R b/tests/testthat/test-tbl-sql.R index 407b6c101..36c24c686 100644 --- a/tests/testthat/test-tbl-sql.R +++ b/tests/testthat/test-tbl-sql.R @@ -65,7 +65,7 @@ test_that("check_from is deprecated", { con <- local_sqlite_connection() DBI::dbExecute(con, "CREATE TABLE x (y)") - expect_snapshot(tbl(con, "x", check_from = FALSE)) + expect_snapshot(out <- tbl(con, "x", check_from = FALSE)) }) # n_groups ---------------------------------------------------------------- diff --git a/tests/testthat/test-verb-summarise.R b/tests/testthat/test-verb-summarise.R index e85cbcde0..bea1cb419 100644 --- a/tests/testthat/test-verb-summarise.R +++ b/tests/testthat/test-verb-summarise.R @@ -107,6 +107,19 @@ test_that("can group transiently using `.by`", { expect_equal(group_vars(out), character()) }) +test_that("across doesn't select columns from `.by` #1493", { + lf <- lazy_frame(g = 1, x = 1) + + out <- lf %>% + summarise( + across(everything(), ~ sum(..x, na.rm = TRUE)), + .by = g + ) + + expect_snapshot(out) + expect_equal(sql_build(out)$select[1], "`g`") +}) + test_that("can't use `.by` with `.groups`", { df <- lazy_frame(x = 1)