add {pre,post}_visit_query to Visitor

[jmhain]

Title says it all.

[coveralls]

Pull Request Test Coverage Report for Build 6934216371

• 0 of 14 (0.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.06%) to 87.645%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/ast/visitor.rs	0	14	0.0%

Totals
Change from base Build 6932119322:	-0.06%
Covered Lines:	17791
Relevant Lines:	20299

---

💛 - Coveralls

[alamb]

Thank you for this contribution @jmhain -- can you please add a test for this new functionality?

[jmhain]

@alamb Sorry for not including tests in the original PR, I just wanted to make sure this functionality was desired first. :) In adding the tests I found a few bugs that resulted from me misunderstanding how the visit macro works. If you include the #[visit] attribute on both the type and the field, then the visitor methods actually end up being called twice. My recent PR that added UNPIVOT support had the same issue, as did the existing PIVOT table factor that I had used as a reference.

I've fixed each of these, added tests that previously reproduced the redundant calls, and added some additional details to the README.md for the derive crate to ensure that others don't make the same error.

[alamb]

Thank you @jmhain -- this looks like a nice contribution to me. @tustvold do you have a few minutes to review this?

[alamb]

why were these removed?

[jmhain]

TableFactor is already annotated with this, so it results in two calls to both the pre_ and post_visit_table_factor for the same table factor, which besides being unnecessary is likely to cause issues in user's code if their visitor doesn't happen to be idempotent.

As an example, this is the output of one of the new tests I added without this change:

 [
     "PRE: STATEMENT: SELECT * FROM monthly_sales PIVOT(SUM(a.amount) FOR a.MONTH IN ('JAN', 'FEB', 'MAR', 'APR')) AS p (c, d) ORDER BY EMPID",
     "PRE: QUERY: SELECT * FROM monthly_sales PIVOT(SUM(a.amount) FOR a.MONTH IN ('JAN', 'FEB', 'MAR', 'APR')) AS p (c, d) ORDER BY EMPID",
     "PRE: TABLE FACTOR: monthly_sales PIVOT(SUM(a.amount) FOR a.MONTH IN ('JAN', 'FEB', 'MAR', 'APR')) AS p (c, d)",
     "PRE: TABLE FACTOR: monthly_sales",
     "PRE: TABLE FACTOR: monthly_sales",
     "PRE: RELATION: monthly_sales",
     "POST: RELATION: monthly_sales",
     "POST: TABLE FACTOR: monthly_sales",
     "POST: TABLE FACTOR: monthly_sales",
     "PRE: EXPR: SUM(a.amount)",
     "PRE: EXPR: a.amount",
     "POST: EXPR: a.amount",
     "POST: EXPR: SUM(a.amount)",
     "POST: TABLE FACTOR: monthly_sales PIVOT(SUM(a.amount) FOR a.MONTH IN ('JAN', 'FEB', 'MAR', 'APR')) AS p (c, d)",
     "PRE: EXPR: EMPID",
     "POST: EXPR: EMPID",
     "POST: QUERY: SELECT * FROM monthly_sales PIVOT(SUM(a.amount) FOR a.MONTH IN ('JAN', 'FEB', 'MAR', 'APR')) AS p (c, d) ORDER BY EMPID",
     "POST: STATEMENT: SELECT * FROM monthly_sales PIVOT(SUM(a.amount) FOR a.MONTH IN ('JAN', 'FEB', 'MAR', 'APR')) AS p (c, d) ORDER BY EMPID",
 ]

Note the duplicate calls for TABLE FACTOR: monthly_sales".

I added a section in the README.md for the derive crate that explains this in more detail: https://github.com/jmhain/sqlparser-rs/blob/visit_query/derive/README.md

I can split this into a separate PR if you'd like since it's separate from adding {pre,post}_visit_query, I just discovered it now because I almost introduced another instance of this same bug here.

[alamb]

Thank you very much @jmhain