From 7666f16c5ed447fa105a9ff1a17266916420d69c Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 2 Mar 2023 15:17:44 +0200 Subject: [PATCH] v16: Online DDL: enforce ALGORITHM=COPY on shadow table (#12522) * Online DDL: enforce ALGORITHM=COPY on shadow table Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * unit tests Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * resolve conflict Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * algorithm format Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * revert change to sql.y Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --------- Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/sqlparser/ast_format.go | 2 +- go/vt/sqlparser/constants.go | 6 ++++++ go/vt/sqlparser/parse_test.go | 6 ++++++ go/vt/vttablet/onlineddl/executor.go | 7 ++++++- go/vt/vttablet/onlineddl/executor_test.go | 24 +++++++++++------------ 5 files changed, 31 insertions(+), 14 deletions(-) diff --git a/go/vt/sqlparser/ast_format.go b/go/vt/sqlparser/ast_format.go index 333ecf1c0df..d8bb1146eb7 100644 --- a/go/vt/sqlparser/ast_format.go +++ b/go/vt/sqlparser/ast_format.go @@ -2200,7 +2200,7 @@ func (node *AddColumns) Format(buf *TrackedBuffer) { // Format formats the node. func (node AlgorithmValue) Format(buf *TrackedBuffer) { - buf.astPrintf(node, "algorithm = %s", string(node)) + buf.astPrintf(node, "algorithm = %#s", string(node)) } // Format formats the node diff --git a/go/vt/sqlparser/constants.go b/go/vt/sqlparser/constants.go index 8331060c0ff..81f2e067563 100644 --- a/go/vt/sqlparser/constants.go +++ b/go/vt/sqlparser/constants.go @@ -65,6 +65,12 @@ const ( AddSequenceStr = "add sequence" AddAutoIncStr = "add auto_increment" + // ALTER TABLE ALGORITHM string. + DefaultStr = "default" + CopyStr = "copy" + InplaceStr = "inplace" + InstantStr = "instant" + // Partition and subpartition type strings HashTypeStr = "hash" KeyTypeStr = "key" diff --git a/go/vt/sqlparser/parse_test.go b/go/vt/sqlparser/parse_test.go index c83cb35a32b..ac3050667b8 100644 --- a/go/vt/sqlparser/parse_test.go +++ b/go/vt/sqlparser/parse_test.go @@ -1238,6 +1238,12 @@ var ( input: "alter table a convert to character set utf32", }, { input: "alter table `By` add column foo int, algorithm = default", + }, { + input: "alter table `By` add column foo int, algorithm = copy", + }, { + input: "alter table `By` add column foo int, algorithm = inplace", + }, { + input: "alter table `By` add column foo int, algorithm = INPLACE", }, { input: "alter table `By` add column foo int, algorithm = instant", }, { diff --git a/go/vt/vttablet/onlineddl/executor.go b/go/vt/vttablet/onlineddl/executor.go index e1788cafcf9..e8410ea8e16 100644 --- a/go/vt/vttablet/onlineddl/executor.go +++ b/go/vt/vttablet/onlineddl/executor.go @@ -102,6 +102,7 @@ var vexecInsertTemplates = []string{ var emptyResult = &sqltypes.Result{} var acceptableDropTableIfExistsErrorCodes = []int{mysql.ERCantFindFile, mysql.ERNoSuchTable} +var copyAlgorithm = sqlparser.AlgorithmValue(sqlparser.CopyStr) var ( ghostOverridePath string @@ -1171,6 +1172,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++ @@ -1179,7 +1183,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, copyAlgorithm}, } alters = append(alters, extraAlterTable) continue @@ -1189,6 +1193,7 @@ func (e *Executor) validateAndEditAlterTableStatement(ctx context.Context, onlin redactedOptions = append(redactedOptions, opt) } alterTable.AlterOptions = redactedOptions + alterTable.AlterOptions = append(alterTable.AlterOptions, copyAlgorithm) return alters, nil } diff --git a/go/vt/vttablet/onlineddl/executor_test.go b/go/vt/vttablet/onlineddl/executor_test.go index e4a8e352026..2cf94be2d20 100644 --- a/go/vt/vttablet/onlineddl/executor_test.go +++ b/go/vt/vttablet/onlineddl/executor_test.go @@ -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 {