Skip to content

ApplySchema: Allow ALTER DATABASE.#5733

Merged
deepthi merged 1 commit intovitessio:masterfrom
planetscale:apply-schema-alter-database
Jan 21, 2020
Merged

ApplySchema: Allow ALTER DATABASE.#5733
deepthi merged 1 commit intovitessio:masterfrom
planetscale:apply-schema-alter-database

Conversation

@enisoc
Copy link
Copy Markdown
Member

@enisoc enisoc commented Jan 18, 2020

This allows using vtctl ApplySchema to execute ALTER DATABASE across all shards of a keyspace.

The main problem was just that ALTER DATABASE had never been added to the sqlparser at all.

@enisoc enisoc requested review from deepthi and systay January 18, 2020 00:28
@enisoc enisoc requested a review from sougou as a code owner January 18, 2020 00:28
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: assert.NoError(t, err)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we no longer know how many object parsedDDLs will contain, we shouldn't specify max size of the array.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@enisoc enisoc force-pushed the apply-schema-alter-database branch 2 times, most recently from 8bb04e9 to 9b748ee Compare January 18, 2020 00:36
@deepthi
Copy link
Copy Markdown
Collaborator

deepthi commented Jan 18, 2020

We only parse the query but never execute it?

@enisoc
Copy link
Copy Markdown
Member Author

enisoc commented Jan 20, 2020

We only parse the query but never execute it?

For ApplySchema, we ultimately just pass through the DDLs to MySQL directly. We only parse them first to check that they're in fact DDLs and to optionally check if they affect "large" tables.

@deepthi
Copy link
Copy Markdown
Collaborator

deepthi commented Jan 20, 2020

We only parse the query but never execute it?

For ApplySchema, we ultimately just pass through the DDLs to MySQL directly. We only parse them first to check that they're in fact DDLs and to optionally check if they affect "large" tables.

I get it now. I can approve once the failing tests have been fixed.

This allows using vtctl ApplySchema to execute ALTER DATABASE across all
shards of a keyspace.

Signed-off-by: Anthony Yeh <enisoc@planetscale.com>
@enisoc enisoc force-pushed the apply-schema-alter-database branch from 9b748ee to b23afc0 Compare January 20, 2020 18:56
@enisoc
Copy link
Copy Markdown
Member Author

enisoc commented Jan 20, 2020

@deepthi Tests are passing now. The default charset is different on my machine vs. in CI so I changed the test to create and drop a new DB instead of altering an existing DB that later gets diffed.

@deepthi deepthi merged commit bbf08d0 into vitessio:master Jan 21, 2020
@enisoc enisoc deleted the apply-schema-alter-database branch January 21, 2020 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants