-
-
Notifications
You must be signed in to change notification settings - Fork 238
implement if [not] exists logic for DDL around views and indexes
#3006
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
c370774
a0f0e0c
27663dd
b8a3c4b
0c4bc3a
32f732e
442c793
2115bf0
3367ff0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,36 @@ import ( | |
| ) | ||
|
|
||
| var ViewScripts = []ScriptTest{ | ||
| { | ||
| Name: "existing views", | ||
| SetUpScript: []string{ | ||
| "create view v as select 1;", | ||
| "create table t (i int);", | ||
| "insert into t values (1);", | ||
| }, | ||
| Assertions: []ScriptTestAssertion{ | ||
| { | ||
| Query: "create view if not exists v as select 2;", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry I didn't notice this while looking at the parser updates... it looks like we are slightly off of MySQL's syntax here... it seems like support for I haven't tested this with MySQL 9.x, so it could be that the documentation is incorrect. I know that syntax is a little inconsistent with |
||
| Expected: []sql.Row{ | ||
| {types.NewOkResult(0)}, | ||
| }, | ||
| }, | ||
| { | ||
| Query: "select * from v;", | ||
| Expected: []sql.Row{{1}}, | ||
| }, | ||
| { | ||
| Query: "create view if not exists t as select 2;", | ||
| Expected: []sql.Row{ | ||
| {types.NewOkResult(0)}, | ||
| }, | ||
| }, | ||
| { | ||
| Query: "select * from t;", | ||
| Expected: []sql.Row{{1}}, | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| Name: "multi database view", | ||
| SetUpScript: []string{ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure MySQL actually supports this syntax, based on their docs:
https://dev.mysql.com/doc/refman/9.3/en/create-index.html
I tested with MySQL 8.4, but didn't try with MySQL 9.x.
That said, I think it's still useful and I'm not opposed to supporting it, although it will likely be hard for customers to discover if it isn't in the MySQL docs. We should just figure out if we want to be consistent with
CREATE TABLE IF NOT EXISTSorCREATE IF NOT EXISTS VIEW, which unfortunately seems to be inconsistent in MySQL.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I noticed this too.
MariaDB has been doing
create view IF NOT EXISTS, which is consistent with all the otherCREATE ... IF NOT EXISTSqueries... it might be possible to do both?So
CREATE [IF NOT EXISTS] VIEW [IF NOT EXISTS] ...?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I downloaded MySQL 9.3 and tested it out; their docs are just messed up.