Skip to content

Commit

Permalink
Ensure all window translation get matching aggregate translation with…
Browse files Browse the repository at this point in the history
… clear errro

Fixes #129
  • Loading branch information
hadley committed Jan 10, 2019
1 parent 4cc738d commit d4d8f2a
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 0 deletions.
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# dbplyr (development version)

* Functions that are only available in a windowed (`mutate()`) query now
throw an error when called in a aggregate (`summarise()`) query (#129)

* `na_if()` is translated to `NULLIF()` for all databases (#211).

* SQL translation (via `partial_eval()`) now correctly interprets the
Expand Down
5 changes: 5 additions & 0 deletions R/translate-sql-base.r
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,11 @@ base_agg <- sql_translator(
sum = sql_aggregate("sum"),
min = sql_aggregate("min"),
max = sql_aggregate("max"),

# first = sql_prefix("FIRST_VALUE", 1),
# last = sql_prefix("LAST_VALUE", 1),
# nth = sql_prefix("NTH_VALUE", 2),

n_distinct = function(...) {
vars <- sql_vector(list(...), parens = FALSE, collapse = ", ")
build_sql("COUNT(DISTINCT ", vars, ")")
Expand Down
16 changes: 16 additions & 0 deletions R/translate-sql-helpers.r
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ sql_variant <- function(scalar = sql_translator(),
))
}

# An ensure that every window function is flagged in aggregate context
missing <- setdiff(ls(window), ls(aggregate))
missing_funs <- map(missing, sql_aggregate_win)
env_bind(aggregate, !!!set_names(missing_funs, missing))

structure(
list(scalar = scalar, aggregate = aggregate, window = window),
class = "sql_variant"
Expand Down Expand Up @@ -187,6 +192,17 @@ sql_aggregate_2 <- function(f) {
}
}

sql_aggregate_win <- function(f) {
force(f)

function(...) {
stop(
"`", f, "()` is only available in a windowed (`mutate()`) context",
call. = FALSE
)
}
}


check_na_rm <- function(f, na.rm) {
if (identical(na.rm, TRUE)) {
Expand Down
9 changes: 9 additions & 0 deletions tests/testthat/test-translate-sql-helpers.r
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,15 @@ test_that("missing window functions create a warning", {
)
})

test_that("missing aggregate functions filled in", {
sim_scalar <- sql_translator()
sim_agg <- sql_translator()
sim_win <- sql_translator(mean = function() {})

trans <- sql_variant(sim_scalar, sim_agg, sim_win)
expect_error(trans$aggregate$mean(), "only available in a window")
})

test_that("output of print method for sql_variant is correct", {
sim_trans <- sql_translator(`+` = sql_infix("+"))
expect_known_output(
Expand Down

0 comments on commit d4d8f2a

Please sign in to comment.