Support COUNT(DISTINCT x) and similar

[benesch]

No description provided.

[coveralls]

Pull Request Test Coverage Report for Build 190

• 34 of 38 (89.47%) changed or added relevant lines in 3 files are covered.
• 4 unchanged lines in 1 file lost coverage.
• Overall coverage remained the same at ?%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/sqlparser.rs	9	11	81.82%
tests/sqlparser_common.rs	12	14	85.71%

Files with Coverage Reduction	New Missed Lines	%
tests/sqlparser_common.rs	4	92.13%

Totals
Change from base Build 183:	0.0%
Covered Lines:
Relevant Lines:	0

---

💛 - Coveralls

[coveralls]

Pull Request Test Coverage Report for Build 208

• 40 of 43 (93.02%) changed or added relevant lines in 3 files are covered.
• 11 unchanged lines in 3 files lost coverage.
• Overall coverage increased (+0.4%) to 89.578%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
tests/sqlparser_common.rs	19	22	86.36%

Files with Coverage Reduction	New Missed Lines	%
src/test_utils.rs	2	89.06%
tests/sqlparser_common.rs	4	93.31%
src/sqlparser.rs	5	88.88%

Totals
Change from base Build 183:	0.4%
Covered Lines:	2905
Relevant Lines:	3243

---

💛 - Coveralls

[nickolay]

The standard says DISTINCT/ALL is allowed in a few ("set") functions, accepting a single argument (<general set function> ::= <set function type> <left paren> [ <set quantifier> ] <value expression> <right paren>), and they may or may not be a window function (i.e. followed by OVER (...)).

This PR extends ASTNode::SQLFunction to hold { name, all, distinct, args, over }, which I think is OK for now, but going forward I think that we might want to switch to using a enum to denote different style of the function arguments (COUNT(*) vs EXTRACT(YEAR FROM val) vs COUNT(DISTINCT val)).

all could be removed for the same reasons as in #76, and I think that putting distinct before args would make the SQLFunction declaration slightly easier to read. LGTM either way, thanks!

[benesch]

This PR extends ASTNode::SQLFunction to hold { name, all, distinct, args, over }, which I think is OK for now, but going forward I think that we might want to switch to using a enum to denote different style of the function arguments (COUNT(*) vs EXTRACT(YEAR FROM val) vs COUNT(DISTINCT val)).

Yeah, agreed that the type system could do more for us here. SQL really blurs the line between function calls and dedicated syntax. (Consider CAST(datum AS type), for example, which is handled with its own ASTNode enum variant already.)

all could be removed for the same reasons as in #76, and I think that putting distinct before args would make the SQLFunction declaration slightly easier to read. LGTM either way, thanks!

Good point, done.