Translatable n_distinct() can only work with single expression

Fixes #101. Fixes #133.

NEWS.md

@@ -1,5 +1,8 @@
 # dbplyr (development version)
 
+* Translation of `n_distinct()` now only supports a single argument 
+  (#101, #133).
+
 * Functions that are only available in a windowed (`mutate()`) query now
   throw an error when called in a aggregate (`summarise()`) query (#129)
 

R/translate-sql-base.r

@@ -222,9 +222,8 @@ base_agg <- sql_translator(
   # 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, ")")
+  n_distinct = function(x) {
+    build_sql("COUNT(DISTINCT ", x, ")")
   }
 )
 
@@ -294,9 +293,8 @@ base_win <- sql_translator(
   n     = function() {
     win_over(sql("COUNT(*)"), win_current_group())
   },
-  n_distinct = function(...) {
-    vars <- sql_vector(list(...), parens = FALSE, collapse = ", ")
-    win_over(build_sql("COUNT(DISTINCT ", vars, ")"), win_current_group())
+  n_distinct = function(x) {
+    win_over(build_sql("COUNT(DISTINCT ", x, ")"), win_current_group())
   },
 
   # Cumulative function are like recycled aggregates except that R names

tests/testthat/test-translate-odbc.R

@@ -21,9 +21,6 @@ test_that("custom aggregators translated correctly", {
   expect_equal(trans(sd(x)),         sql("STDDEV_SAMP(`x`)"))
   expect_equal(trans(count()),       sql("COUNT(*)"))
   expect_equal(trans(n()),           sql("COUNT(*)"))
-  expect_equal(trans(n_distinct(x)), sql("COUNT(DISTINCT `x`)"))
-
-
 })
 
 test_that("custom window functions translated correctly", {
@@ -35,9 +32,6 @@ test_that("custom window functions translated correctly", {
   expect_equal(trans(sd(x, na.rm = TRUE)),         sql("STDDEV_SAMP(`x`) OVER ()"))
   expect_equal(trans(count()),       sql("COUNT(*) OVER ()"))
   expect_equal(trans(n()),           sql("COUNT(*) OVER ()"))
-  expect_equal(trans(n_distinct(x)), sql("COUNT(DISTINCT `x`) OVER ()"))
-
-
 })
 
 test_that("queries translate correctly", {

tests/testthat/test-translate.r

@@ -64,15 +64,16 @@ test_that("%in% with empty vector", {
   expect_equal(translate_sql(x %in% !!integer()), sql('FALSE'))
 })
 
-test_that("n_distinct can take multiple values", {
+test_that("n_distinct(x) translated to COUNT(distinct, x)", {
   expect_equal(
     translate_sql(n_distinct(x), window = FALSE),
     sql('COUNT(DISTINCT "x")')
   )
   expect_equal(
-    translate_sql(n_distinct(x, y), window = FALSE),
-    sql('COUNT(DISTINCT "x", "y")')
+    translate_sql(n_distinct(x), window = TRUE),
+    sql('COUNT(DISTINCT "x") OVER ()')
   )
+  expect_error(translate_sql(n_distinct(x, y), window = FALSE), "unused argument")
 })
 
 test_that("na_if is translated to NULLIF (#211)", {