Skip to content
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

Normalizing Online-DDL queries #7153

Merged
merged 12 commits into from
Dec 16, 2020

Conversation

shlomi-noach
Copy link
Contributor

@shlomi-noach shlomi-noach commented Dec 10, 2020

Backport

NO

Status

REAFY

Description

In this PR we normalize some DDL queries sent as Online DDL:

  • CREATE INDEX formats onto ALTER TABLE: this is because gh-ost and pt-osc only know how to handle ALTER TABLE statements. As there is no specific advantage to using CREATE INDEX over the equivalent CREATE TABLE... ADD INDEX we choose to have CreateIndex.Format() function export as ALTER TABLE
  • DROP TABLE <multi> - exploded into multiple single-table DROP statements

Related Issue(s)

#6926 (comment)

Todos

  • Tests
  • Documentation

Impacted Areas in Vitess

List general components of the application that this PR will affect:

  • Query Serving
  • VReplication
  • Cluster Management
  • Build

Notes

@GuptaManan100 @systay please notice the following test from parse_test.go:

		input: "create index a on b (col1) using btree key_block_size=12 with parser 'a' comment 'string' algorithm inplace lock none",
		// This is actually an incorrect output: should be the one commented
		output: "alter table b add index a (col1) using btree key_block_size 12 with parser 'a' comment 'string' algorithm inplace lock none",
		// output: "alter table b add index a using btree (col1) key_block_size=12 with parser 'a' comment 'string' algorithm inplace lock none",

The output is incorrect; still to understand why, I think the problem is with the parser, not with the Format. FWIW MySQL itself is unhappy about that complex CREAT EINDEX query.

- CREATE INDEX formats onto ALTER TABLE
- DROP TABLE <multi> - exploded into multiple single-table DROP statements

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
"drop database t": {isError: true},
}
for query, expect := range tests {
normalized, err := NormalizeOnlineDDL(query)
Copy link
Collaborator

@systay systay Dec 10, 2020

Choose a reason for hiding this comment

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

nit:

for query, expect := range tests {
	t.Run(query, func(t *testing.T) {
...
	})
}

sub tests makes it look good in test output

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TIL actually

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@shlomi-noach
Copy link
Contributor Author

Still have some tests to fix.

Signed-off-by: GuptaManan100 <manan@planetscale.com>
Copy link
Member

@GuptaManan100 GuptaManan100 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
@shlomi-noach
Copy link
Contributor Author

Documentation: vitessio/website#638

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
@shlomi-noach shlomi-noach merged commit 54e01f0 into vitessio:master Dec 16, 2020
@shlomi-noach shlomi-noach deleted the online-ddl-normalize-queries branch December 16, 2020 07:21
@askdba askdba added this to the v9.0 milestone Dec 23, 2020
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.

4 participants