From 1b58b6899c72a570ae8b68df6f91557d045009f2 Mon Sep 17 00:00:00 2001
From: Hadley Wickham <h.wickham@gmail.com>
Date: Thu, 10 Jan 2019 10:05:37 -0600
Subject: [PATCH] Translatable n_distinct() can only work with single
 expression

Fixes #101. Fixes #133.
---
 NEWS.md                              |  3 +++
 R/translate-sql-base.r               | 10 ++++------
 tests/testthat/test-translate-odbc.R |  6 ------
 tests/testthat/test-translate.r      |  7 ++++---
 4 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index 875214e3e..404018469 100644
--- a/NEWS.md
+++ b/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)
 
diff --git a/R/translate-sql-base.r b/R/translate-sql-base.r
index 623840924..873bbb4a9 100644
--- a/R/translate-sql-base.r
+++ b/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
diff --git a/tests/testthat/test-translate-odbc.R b/tests/testthat/test-translate-odbc.R
index c28a9984e..7046b6b06 100644
--- a/tests/testthat/test-translate-odbc.R
+++ b/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", {
diff --git a/tests/testthat/test-translate.r b/tests/testthat/test-translate.r
index c67101513..2eff08b5a 100644
--- a/tests/testthat/test-translate.r
+++ b/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)", {