sql: ALTER TABLE ADD {,COLUMN,CONSTRAINT} support #2435
Conversation
tamird
changed the title
|
|
||
| // AlterTable creates a table. | ||
| // Privileges: CREATE on database. | ||
| // Notes: postgres/mysql require CREATE on database. |
There was a problem hiding this comment.
Looks like you're checking for CREATE privilege on the table. Perhaps I'm not understanding the notation here. @mberhault?
|
LGTM. What conflicts with #2434 are you worried about? That change will remove the ability to create a non-unique index using |
|
LGTM |
|
@petermattis can you take another look? it took some refactoring, but this implementation now shares backfill code with CREATE INDEX. |
| var b client.Batch | ||
| // Get all the rows affected. | ||
| // TODO(vivek): Avoid going through Select. | ||
| // TODO(tamird): Support partial indexes? |
There was a problem hiding this comment.
I realize this is a cut&paste comment. I'm not sure if we'll ever support partial indexes. Have you seen them used in practice?
There was a problem hiding this comment.
Never seen them used since MySQL doesn't support them, but I've seen cases where we would have used them if they were available. They're not strictly necessary, but nice to have.
|
LGTM |
Fixed a bug that permitted multiple primary keys on table creation.
sql: ALTER TABLE ADD {,COLUMN,CONSTRAINT} support
raft: Introduce MultiNode.
Work toward #2035.
There are potential conflicts between this, #2430, and #2434.
I've intentionally avoided test cases which exercise constraint-as-index.