Support DROP [TABLE|VIEW]

[benesch]

No description provided.

[coveralls]

Pull Request Test Coverage Report for Build 203

• 94 of 99 (94.95%) changed or added relevant lines in 3 files are covered.
• 7 unchanged lines in 2 files lost coverage.
• Overall coverage increased (+0.5%) to 89.646%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
tests/sqlparser_common.rs	49	51	96.08%
src/sqlparser.rs	26	29	89.66%

Files with Coverage Reduction	New Missed Lines	%
src/test_utils.rs	2	89.06%
src/sqlparser.rs	5	88.71%

Totals
Change from base Build 183:	0.5%
Covered Lines:	2961
Relevant Lines:	3303

---

💛 - Coveralls

[nickolay]

I think this adds support for parsing DROP { TABLE | VIEW } [ IF EXISTS ] <object_name>[, <object_name>...] [ { CASCADE | RESTRICT } ], where

• CASCADE/RESTRICT is standard (the default, when omitted, is RESTRICT)
• IF EXISTS and multiple object names in one statement are non-standard, but are supported in various DBs (e.g. Postgresql, MSSQL)

I had a less complete implementation in andygrove@1021d39 - which I'll remove. I had one variant in SQLStatement (SQLDrop) with the object type (TABLE or VIEW) stored in a field. Given that the majority of DROP statements in the spec are defined as either DROP <OBJECT_TYPE> <drop behavior> or simply DROP <OBJECT_TYPE>, perhaps it would be simpler to do it like I did? That way you wouldn't need the "_internal"/"_inner" methods.

Please run cargo fmt if you choose to update the PR.

(Also note that this has a (trivial) conflict with #74)

[nickolay]

comma_separated_string(&self.names), seems to work here.

[benesch]

Good point. Done.

[nickolay]

Like with SELECT ALL (#76) I wonder if it makes sense to store the restrict in the AST, given it's the default.

[benesch]

Agreed, fixed.

[benesch]

I think this adds support for parsing DROP { TABLE | VIEW } [ IF EXISTS ] <object_name>[, <object_name>...] [ { CASCADE | RESTRICT } ], where

• CASCADE/RESTRICT is standard (the default, when omitted, is RESTRICT)
• IF EXISTS and multiple object names in one statement are non-standard, but are supported in various DBs (e.g. Postgresql, MSSQL)

Yes, that's exactly right.

I had a less complete implementation in 1021d39 - which I'll remove. I had one variant in SQLStatement (SQLDrop) with the object type (TABLE or VIEW) stored in a field. Given that the majority of DROP statements in the spec are defined as either DROP <OBJECT_TYPE> <drop behavior> or simply DROP <OBJECT_TYPE>, perhaps it would be simpler to do it like I did? That way you wouldn't need the "_internal"/"_inner" methods.

Oh, interesting. We didn't notice that before writing this implementation. Definitely not opposed to having SQLDrop store the object type (table/view), rather than vice-versa, though I'd prefer to have a proper enum rather than a string type, so that it's possible to exhaustively handle all the cases. Thoughts?

Please run cargo fmt if you choose to update the PR.

Done.

[benesch]

Oh, interesting. We didn't notice that before writing this implementation. Definitely not opposed to having SQLDrop store the object type (table/view), rather than vice-versa, though I'd prefer to have a proper enum rather than a string type, so that it's possible to exhaustively handle all the cases. Thoughts?

I decided I like this a lot, and adjusted the PR accordingly.

[nickolay]

Great, thanks! I've asked @andygrove if he wants to take a look at this and 72, 73, 76 before merging. I hope to get to the other PRs "soon":)