Fix distinct() for SQL sources

[krlmlr]

Without columns:

• .keep_all is ignored if no columns are selected

With columns:

• .keep_all = FALSE is implemented using a grouped select
• .keep_all = TRUE still raises an error, because DISTINCT syntax seems to vary between SQL dialects

Adapted tests.

Fixes #1937.

[codecov-io]

Current coverage is 61.68%

Merging #1942 into master will increase coverage by 0.04%

@@             master      #1942   diff @@
==========================================
  Files           188        188          
  Lines          7459       7468     +9   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           4598       4607     +9   
  Misses         2861       2861          
  Partials          0          0          

Powered by Codecov. Last updated by 284b91d...6afb0b0

[hadley]

Would you mind adding a test for the SQL generation too?

[krlmlr]

What do you have in mind -- something like the tests in test-sql-render.R?

[hadley]

Yeah exactly - when reviewing this PR it would help if I had a concrete example of the SQL that is generated.

[krlmlr]

Done. On second thought, SELECT DISTINCT y might be a simpler query for the second case; not sure how portable it is though.

[hadley]

Thanks - that seems like a reasonable approach to me.

[lock]

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/