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.

Sign up for GitHub

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

v16: Online DDL: enforce ALGORITHM=COPY on shadow table #12522

Merged
merged 6 commits into from
Mar 2, 2023
Merged

v16: Online DDL: enforce ALGORITHM=COPY on shadow table #12522

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2255722
Online DDL: enforce ALGORITHM=COPY on shadow table
shlomi-noach Feb 28, 2023
bb95642
unit tests
shlomi-noach Feb 28, 2023
a7cb98f
resolve conflict
shlomi-noach Mar 1, 2023
4b2f4c2
algorithm format
shlomi-noach Mar 1, 2023
d7d6d85
Merge branch 'release-16.0' into online-ddl-algorithm-copy-v16
shlomi-noach Mar 1, 2023
d261748
revert change to sql.y
shlomi-noach Mar 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion go/vt/vttablet/onlineddl/executor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1167,6 +1167,9 @@ func (e *Executor) validateAndEditAlterTableStatement(ctx context.Context, onlin
for i := range alterTable.AlterOptions {
opt := alterTable.AlterOptions[i]
switch opt := opt.(type) {
case sqlparser.AlgorithmValue:
// we do not pass ALGORITHM. We choose our own ALGORITHM.
continue
case *sqlparser.AddIndexDefinition:
if opt.IndexDefinition.Info.Fulltext {
countAddFullTextStatements++
Expand All @@ -1175,7 +1178,7 @@ func (e *Executor) validateAndEditAlterTableStatement(ctx context.Context, onlin
// in the same statement
extraAlterTable := &sqlparser.AlterTable{
Table: alterTable.Table,
AlterOptions: []sqlparser.AlterOption{opt},
AlterOptions: []sqlparser.AlterOption{opt, sqlparser.AlgorithmValue("COPY")},
Copy link
Member

Choose a reason for hiding this comment

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

Is it worth declaring the algorithm values as constants versus hard-coded strings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made as constants in sql.y and sqlparser level

}
alters = append(alters, extraAlterTable)
continue
Expand All @@ -1185,6 +1188,7 @@ func (e *Executor) validateAndEditAlterTableStatement(ctx context.Context, onlin
redactedOptions = append(redactedOptions, opt)
}
alterTable.AlterOptions = redactedOptions
alterTable.AlterOptions = append(alterTable.AlterOptions, sqlparser.AlgorithmValue("COPY"))
return alters, nil
}

Expand Down
24 changes: 12 additions & 12 deletions go/vt/vttablet/onlineddl/executor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,51 +182,51 @@ func TestValidateAndEditAlterTableStatement(t *testing.T) {
}{
{
alter: "alter table t add column i int",
expect: []string{"alter table t add column i int"},
expect: []string{"alter table t add column i int, algorithm = COPY"},
},
{
alter: "alter table t add column i int, add fulltext key name1_ft (name1)",
expect: []string{"alter table t add column i int, add fulltext key name1_ft (name1)"},
expect: []string{"alter table t add column i int, add fulltext key name1_ft (name1), algorithm = COPY"},
},
{
alter: "alter table t add column i int, add fulltext key name1_ft (name1), add fulltext key name2_ft (name2)",
expect: []string{"alter table t add column i int, add fulltext key name1_ft (name1)", "alter table t add fulltext key name2_ft (name2)"},
expect: []string{"alter table t add column i int, add fulltext key name1_ft (name1), algorithm = COPY", "alter table t add fulltext key name2_ft (name2), algorithm = COPY"},
},
{
alter: "alter table t add fulltext key name0_ft (name0), add column i int, add fulltext key name1_ft (name1), add fulltext key name2_ft (name2)",
expect: []string{"alter table t add fulltext key name0_ft (name0), add column i int", "alter table t add fulltext key name1_ft (name1)", "alter table t add fulltext key name2_ft (name2)"},
expect: []string{"alter table t add fulltext key name0_ft (name0), add column i int, algorithm = COPY", "alter table t add fulltext key name1_ft (name1), algorithm = COPY", "alter table t add fulltext key name2_ft (name2), algorithm = COPY"},
},
{
alter: "alter table t add constraint check (id != 1)",
expect: []string{"alter table t add constraint chk_aulpn7bjeortljhguy86phdn9 check (id != 1)"},
expect: []string{"alter table t add constraint chk_aulpn7bjeortljhguy86phdn9 check (id != 1), algorithm = COPY"},
},
{
alter: "alter table t add constraint t_chk_1 check (id != 1)",
expect: []string{"alter table t add constraint chk_1_aulpn7bjeortljhguy86phdn9 check (id != 1)"},
expect: []string{"alter table t add constraint chk_1_aulpn7bjeortljhguy86phdn9 check (id != 1), algorithm = COPY"},
},
{
alter: "alter table t add constraint some_check check (id != 1)",
expect: []string{"alter table t add constraint some_check_aulpn7bjeortljhguy86phdn9 check (id != 1)"},
expect: []string{"alter table t add constraint some_check_aulpn7bjeortljhguy86phdn9 check (id != 1), algorithm = COPY"},
},
{
alter: "alter table t add constraint some_check check (id != 1), add constraint another_check check (id != 2)",
expect: []string{"alter table t add constraint some_check_aulpn7bjeortljhguy86phdn9 check (id != 1), add constraint another_check_4fa197273p3w96267pzm3gfi3 check (id != 2)"},
expect: []string{"alter table t add constraint some_check_aulpn7bjeortljhguy86phdn9 check (id != 1), add constraint another_check_4fa197273p3w96267pzm3gfi3 check (id != 2), algorithm = COPY"},
},
{
alter: "alter table t add foreign key (parent_id) references onlineddl_test_parent (id) on delete no action",
expect: []string{"alter table t add constraint fk_6fmhzdlya89128u5j3xapq34i foreign key (parent_id) references onlineddl_test_parent (id) on delete no action"},
expect: []string{"alter table t add constraint fk_6fmhzdlya89128u5j3xapq34i foreign key (parent_id) references onlineddl_test_parent (id) on delete no action, algorithm = COPY"},
},
{
alter: "alter table t add constraint myfk foreign key (parent_id) references onlineddl_test_parent (id) on delete no action",
expect: []string{"alter table t add constraint myfk_6fmhzdlya89128u5j3xapq34i foreign key (parent_id) references onlineddl_test_parent (id) on delete no action"},
expect: []string{"alter table t add constraint myfk_6fmhzdlya89128u5j3xapq34i foreign key (parent_id) references onlineddl_test_parent (id) on delete no action, algorithm = COPY"},
},
{
alter: "alter table t add constraint t_fk_1 foreign key (parent_id) references onlineddl_test_parent (id) on delete no action",
expect: []string{"alter table t add constraint fk_1_6fmhzdlya89128u5j3xapq34i foreign key (parent_id) references onlineddl_test_parent (id) on delete no action"},
expect: []string{"alter table t add constraint fk_1_6fmhzdlya89128u5j3xapq34i foreign key (parent_id) references onlineddl_test_parent (id) on delete no action, algorithm = COPY"},
},
{
alter: "alter table t add constraint t_fk_1 foreign key (parent_id) references onlineddl_test_parent (id) on delete no action, add constraint some_check check (id != 1)",
expect: []string{"alter table t add constraint fk_1_6fmhzdlya89128u5j3xapq34i foreign key (parent_id) references onlineddl_test_parent (id) on delete no action, add constraint some_check_aulpn7bjeortljhguy86phdn9 check (id != 1)"},
expect: []string{"alter table t add constraint fk_1_6fmhzdlya89128u5j3xapq34i foreign key (parent_id) references onlineddl_test_parent (id) on delete no action, add constraint some_check_aulpn7bjeortljhguy86phdn9 check (id != 1), algorithm = COPY"},
},
}
for _, tc := range tt {
Expand Down