diff --git a/go/vt/schemadiff/table.go b/go/vt/schemadiff/table.go index f6b7677e8bf..f58faa06310 100644 --- a/go/vt/schemadiff/table.go +++ b/go/vt/schemadiff/table.go @@ -775,15 +775,34 @@ func (c *CreateTableEntity) TableDiff(other *CreateTableEntity, hints *DiffHints } } tableSpecHasChanged := len(alterTable.AlterOptions) > 0 || alterTable.PartitionOption != nil || alterTable.PartitionSpec != nil + + newAlterTableEntityDiff := func(alterTable *sqlparser.AlterTable) *AlterTableEntityDiff { + d := &AlterTableEntityDiff{alterTable: alterTable, from: c, to: other} + + var algorithmValue sqlparser.AlgorithmValue + + switch hints.AlterTableAlgorithmStrategy { + case AlterTableAlgorithmStrategyCopy: + algorithmValue = sqlparser.AlgorithmValue("COPY") + case AlterTableAlgorithmStrategyInplace: + algorithmValue = sqlparser.AlgorithmValue("INPLACE") + case AlterTableAlgorithmStrategyInstant: + algorithmValue = sqlparser.AlgorithmValue("INSTANT") + } + if algorithmValue != "" { + alterTable.AlterOptions = append(alterTable.AlterOptions, algorithmValue) + } + return d + } if tableSpecHasChanged { - parentAlterTableEntityDiff = &AlterTableEntityDiff{alterTable: alterTable, from: c, to: other} + parentAlterTableEntityDiff = newAlterTableEntityDiff(alterTable) } for _, superfluousFulltextKey := range superfluousFulltextKeys { alterTable := &sqlparser.AlterTable{ Table: c.CreateTable.Table, AlterOptions: []sqlparser.AlterOption{superfluousFulltextKey}, } - diff := &AlterTableEntityDiff{alterTable: alterTable, from: c, to: other} + diff := newAlterTableEntityDiff(alterTable) // if we got superfluous fulltext keys, that means the table spec has changed, ie // parentAlterTableEntityDiff is not nil parentAlterTableEntityDiff.addSubsequentDiff(diff) @@ -793,7 +812,7 @@ func (c *CreateTableEntity) TableDiff(other *CreateTableEntity, hints *DiffHints Table: c.CreateTable.Table, PartitionSpec: partitionSpec, } - diff := &AlterTableEntityDiff{alterTable: alterTable, from: c, to: other} + diff := newAlterTableEntityDiff(alterTable) if parentAlterTableEntityDiff == nil { parentAlterTableEntityDiff = diff } else { @@ -1990,6 +2009,8 @@ func (c *CreateTableEntity) apply(diff *AlterTableEntityDiff) error { c.TableSpec.Options = append(c.TableSpec.Options, option) }() } + case sqlparser.AlgorithmValue: + // silently ignore. This has an operational effect on the MySQL engine, but has no semantical effect. default: return &UnsupportedApplyOperationError{Statement: sqlparser.CanonicalString(opt)} } diff --git a/go/vt/schemadiff/table_test.go b/go/vt/schemadiff/table_test.go index 4f5fd8424db..18fce4ec759 100644 --- a/go/vt/schemadiff/table_test.go +++ b/go/vt/schemadiff/table_test.go @@ -45,6 +45,7 @@ func TestCreateTableDiff(t *testing.T) { colrename int constraint int charset int + algorithm int }{ { name: "identical", @@ -1114,6 +1115,31 @@ func TestCreateTableDiff(t *testing.T) { diff: "alter table t1 comment ''", cdiff: "ALTER TABLE `t1` COMMENT ''", }, + // algorithm + { + name: "algorithm: COPY", + from: "create table t1 (`id` int primary key)", + to: "create table t2 (id int primary key, `i` int not null default 0)", + diff: "alter table t1 add column i int not null default 0, algorithm = COPY", + cdiff: "ALTER TABLE `t1` ADD COLUMN `i` int NOT NULL DEFAULT 0, ALGORITHM = COPY", + algorithm: AlterTableAlgorithmStrategyCopy, + }, + { + name: "algorithm: INPLACE", + from: "create table t1 (`id` int primary key)", + to: "create table t2 (id int primary key, `i` int not null default 0)", + diff: "alter table t1 add column i int not null default 0, algorithm = INPLACE", + cdiff: "ALTER TABLE `t1` ADD COLUMN `i` int NOT NULL DEFAULT 0, ALGORITHM = INPLACE", + algorithm: AlterTableAlgorithmStrategyInplace, + }, + { + name: "algorithm: INSTANT", + from: "create table t1 (`id` int primary key)", + to: "create table t2 (id int primary key, `i` int not null default 0)", + diff: "alter table t1 add column i int not null default 0, algorithm = INSTANT", + cdiff: "ALTER TABLE `t1` ADD COLUMN `i` int NOT NULL DEFAULT 0, ALGORITHM = INSTANT", + algorithm: AlterTableAlgorithmStrategyInstant, + }, } standardHints := DiffHints{} for _, ts := range tt { @@ -1140,6 +1166,7 @@ func TestCreateTableDiff(t *testing.T) { hints.ColumnRenameStrategy = ts.colrename hints.FullTextKeyStrategy = ts.fulltext hints.TableCharsetCollateStrategy = ts.charset + hints.AlterTableAlgorithmStrategy = ts.algorithm alter, err := c.Diff(other, &hints) require.Equal(t, len(ts.diffs), len(ts.cdiffs)) diff --git a/go/vt/schemadiff/types.go b/go/vt/schemadiff/types.go index 365c5cfb7fb..1f1186b41bd 100644 --- a/go/vt/schemadiff/types.go +++ b/go/vt/schemadiff/types.go @@ -94,6 +94,13 @@ const ( TableCharsetCollateIgnoreAlways ) +const ( + AlterTableAlgorithmStrategyNone int = iota + AlterTableAlgorithmStrategyInstant + AlterTableAlgorithmStrategyInplace + AlterTableAlgorithmStrategyCopy +) + // DiffHints is an assortment of rules for diffing entities type DiffHints struct { StrictIndexOrdering bool @@ -104,4 +111,5 @@ type DiffHints struct { TableRenameStrategy int FullTextKeyStrategy int TableCharsetCollateStrategy int + AlterTableAlgorithmStrategy int } diff --git a/go/vt/sidecardb/sidecardb.go b/go/vt/sidecardb/sidecardb.go index 4c3ebf6caab..840fb16b849 100644 --- a/go/vt/sidecardb/sidecardb.go +++ b/go/vt/sidecardb/sidecardb.go @@ -306,6 +306,7 @@ func (si *schemaInit) getCurrentSchema(tableName string) (string, error) { func (si *schemaInit) findTableSchemaDiff(tableName, current, desired string) (string, error) { hints := &schemadiff.DiffHints{ TableCharsetCollateStrategy: schemadiff.TableCharsetCollateIgnoreAlways, + AlterTableAlgorithmStrategy: schemadiff.AlterTableAlgorithmStrategyCopy, } diff, err := schemadiff.DiffCreateTablesQueries(current, desired, hints) if err != nil { diff --git a/go/vt/sidecardb/sidecardb_test.go b/go/vt/sidecardb/sidecardb_test.go index 136d814f12c..1f6b58ac493 100644 --- a/go/vt/sidecardb/sidecardb_test.go +++ b/go/vt/sidecardb/sidecardb_test.go @@ -20,6 +20,8 @@ import ( "context" "testing" + "vitess.io/vitess/go/vt/sqlparser" + "github.com/stretchr/testify/require" "vitess.io/vitess/go/mysql/fakesqldb" @@ -113,3 +115,37 @@ func TestValidateSchema(t *testing.T) { }) } } + +// TestAlterTableAlgorithm confirms that we use ALGORITHM=COPY during alter tables +func TestAlterTableAlgorithm(t *testing.T) { + type testCase struct { + testName string + tableName string + currentSchema string + desiredSchema string + } + testCases := []testCase{ + {"add column", "t1", "create table if not exists _vt.t1(i int)", "create table if not exists _vt.t1(i int, i1 int)"}, + {"modify column", "t1", "create table if not exists _vt.t1(i int)", "create table if not exists _vt.t(i float)"}, + } + si := &schemaInit{} + copyAlgo := sqlparser.AlgorithmValue("COPY") + for _, tc := range testCases { + t.Run(tc.testName, func(t *testing.T) { + diff, err := si.findTableSchemaDiff(tc.tableName, tc.currentSchema, tc.desiredSchema) + require.NoError(t, err) + stmt, err := sqlparser.Parse(diff) + require.NoError(t, err) + alterTable, ok := stmt.(*sqlparser.AlterTable) + require.True(t, ok) + require.NotNil(t, alterTable) + var alterAlgo sqlparser.AlterOption + for i, opt := range alterTable.AlterOptions { + if _, ok := opt.(sqlparser.AlgorithmValue); ok { + alterAlgo = alterTable.AlterOptions[i] + } + } + require.Equal(t, copyAlgo, alterAlgo) + }) + } +}