WIP: Support CREATE/DROP INDEX#14
Conversation
|
|
| on_name: ObjectName, | ||
| /// expressions that form part of the index key | ||
| key_parts: Vec<Expr>, | ||
| }, |
There was a problem hiding this comment.
Nit: comments should have a space after the slash and start with an upper case letter. (I try not to care too much about formatting in general, but it matters here because this will be upstreamed, and the less things that can go wrong in the review when we upstream, the better.)
| Source => "SOURCES", | ||
| Sink => "SINKS", | ||
| Index => unreachable!(), | ||
| } |
There was a problem hiding this comment.
Just flagging this in case you’ve forgotten about it.
There was a problem hiding this comment.
It will not be reachable until SHOW INDEXES is supported. Should I just change it to "INDEXES" preemptively?
There was a problem hiding this comment.
Oh, duh, of course. Apologies; I didn't look hard enough at where this code was. Leaving it like this is fine!
| }) { | ||
| Err(ParserError::ParserError(format!( | ||
| "Expressions must be enclosed in parentheses." | ||
| ))) |
There was a problem hiding this comment.
I would actually drop this! The fact that you have to surround complicated expressions with parentheses is a limitation of the PostgreSQL parser. Since we don’t have that limitation, and since it’s less code to not introduce that limitation, the rule of thumb is to not have the limitation.
|
And yeah, we should support SHOW INDEX, but no need to do so in this PR. |
benesch
left a comment
benesch
left a comment
There was a problem hiding this comment.
Amazing tests, as always!
3 participants
Part of the work involved in MaterializeInc/materialize#218
Basic support for the
CREATE INDEXandDROP INDEXsyntax.Note that index creation and management syntax is not part of the SQL standard.
Syntax for CREATE INDEX
CREATE INDEX index_name ON table_name(key_part, …)
key_part: {col_name | (expr)}
This is a subset of the overlap between PostgreSQL and MySQL. Links to the full syntax for both DBs are below in case we think that we may want to support some additional features or allow sqlparser to be able to support future implementations of the certain features.
https://www.postgresql.org/docs/12/sql-createindex.html
https://dev.mysql.com/doc/refman/8.0/en/create-index.html
In PostgreSQL, all expressions other than column names (without qualifiers) and functions must have parentheses around them, and I have followed that parsing rule in the PR. I'm not sure if MySQL exempts functions from the parentheses requirement.
Syntax for DROP INDEX
DROP INDEX [IF EXISTS] index_name [, …] (CASCADE | RESTRICT)
This is a subset of what PostgreSQL supports and a superset of what MySQL supports. This syntax is motivated by it being the default state for DROP and it being more of a hassle to not support the options.
Question: Do we want to support SHOW INDEX/INDEXES/KEYS?
Currently in this branch
SHOW INDEXis not supported.SHOW INDEXis supported in MySQL (and CockroachDB), but it is not supported in PostgreSQL, Oracle, or Microsoft SQL Server.The MySQL syntax is here:
https://dev.mysql.com/doc/refman/8.0/en/show-index.html
Since indexes belong to tables, it seems like supporting
SHOW INDEXwould involve reworking theStatement::ShowObjectsorObjectTypeenums.