Consolidate representation of function calls, remove AggregateExpressionWithFilter, ArraySubquery, ListAgg and ArrayAgg

[jmhain]

This addresses a handful of issues and redundancies related to our representation of function calls.

• I split off a new FunctionArguments enum with three options:
  • A regular argument list with a list of additional clauses
  • No argument list - for representing things like CURRENT_TIMESTAMP. This is better than the special bit we had because the latter allows for many invalid AST states.
  • A subquery. This is used to losslessly represent Snowflake's support for unparenthesized subqueries as function arguments, and allowed removal of the special ArraySubquery expression variant.
• Reimplemented the fix from Support BigQuery window function null treatment #1239 to take advantage of the new structure.
• I added WITHIN GROUP support to Function, which makes functions like percentile_cont(...) within group (...) on Postgres work.
• I removed the special ListAgg and ArrayAgg expression variants and subsumed them into Function
• Removed the redundant AggregateExpressionWithFilter expression variant. This was only needed before because of ArrayAgg / ListAgg being separate expression nodes.
• On BigQuery, ARRAY_CONCAT_AGG and STRING_AGG support the same set of options as ARRAY_AGG, and we can parse those now.
• The ALL in SELECT COUNT(ALL column) is now preserved in roundtrips.

This lays the groundwork for adding additional special function argument clauses like bigquery's ANY_VALUE(foo HAVING MIN value).

[coveralls]

Pull Request Test Coverage Report for Build 8943406713

Details

• 471 of 506 (93.08%) changed or added relevant lines in 14 files are covered.
• 25 unchanged lines in 6 files lost coverage.
• Overall coverage decreased (-0.03%) to 89.172%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
tests/sqlparser_bigquery.rs	9	10	90.0%
tests/sqlparser_mssql.rs	5	6	83.33%
src/ast/mod.rs	37	39	94.87%
src/parser/mod.rs	87	91	95.6%
tests/sqlparser_common.rs	192	219	87.67%

Files with Coverage Reduction	New Missed Lines	%
src/dialect/snowflake.rs	1	91.79%
tests/sqlparser_mssql.rs	1	92.76%
src/ast/mod.rs	2	80.78%
src/dialect/mod.rs	3	76.98%
src/parser/mod.rs	4	92.81%
tests/sqlparser_common.rs	14	89.46%

Totals
Change from base Build 8943225733:	-0.03%
Covered Lines:	24178
Relevant Lines:	27114

---

💛 - Coveralls

[iffyio]

LGTM! Very nice with the new representation, thanks @jmhain!

cc @alamb

[alamb]

👏 -- thank you @jmhain -- this is really nice. While it will be a non trivial change for some downstream crates I think it will be for the better.

Also, thank you for the reviews @iffyio and @universalmind303

🚀

[alamb]

❤️

[alamb]

These comments are really lovely. Thank you @jmhain

[tisonkun]

How do I get this property now then?

[tisonkun]

I can see DuplicateTreatment now. But it can cause downstream to restructure the handling code to adjust these structural changes.

Trying it out ...

[alamb]

Let us know if there is something wrong that can't be handled -- I can make another release with a fix if necessary

[tisonkun]

I'm trying to upgrade sqlparser to 0.46.0 in datafusion this morning but found that except my breaking changes on Insert and Delete struct, it includes others like this one and the JsonAccess refactor. So I stop the work until I can have time to understand the changes :P

[jmhain]

Let me know if there's any way I can help...

[tisonkun]

@jmhain Thanks for offering the help. I just created apache/datafusion#10392 that you can pull locally and run cargo build to see the compilation failure and see if you have the idea to fix it up.

Welcome to leave a comment on that PR or create a PR target to that PR. I'm actively responding.

[jmhain]

This is what I've got so far that I'm reasonably sure is correct without more context on DataFusion: apache/datafusion@261f346

Out of time for now but I'll try to take a second look later tonight at the remaining JSON access changes needed