Online DDL: more support for INSTANT DDL (#11591)

* Online DDL: support instant DDL for enum changes; strategy flag now called '--prefer-instant-ddl'

Signed-off-by: Shlomi Noach <[email protected]>

* typo

Signed-off-by: Shlomi Noach <[email protected]>

Signed-off-by: Shlomi Noach <[email protected]>

go/mysql/flavor.go

@@ -49,6 +49,7 @@ const (
 	InstantAddDropVirtualColumnFlavorCapability
 	InstantAddDropColumnFlavorCapability
 	InstantChangeColumnDefaultFlavorCapability
+	InstantExpandEnumCapability
 	MySQLJSONFlavorCapability
 	MySQLUpgradeInServerFlavorCapability
 	DynamicRedoLogCapacityFlavorCapability // supported in MySQL 8.0.30 and above: https://dev.mysql.com/doc/relnotes/mysql/8.0/en/news-8-0-30.html

go/mysql/flavor_mysql.go

@@ -390,6 +390,7 @@ func (mysqlFlavor80) baseShowTablesWithSizes() string {
 func (mysqlFlavor80) supportsCapability(serverVersion string, capability FlavorCapability) (bool, error) {
 	switch capability {
 	case InstantDDLFlavorCapability,
+		InstantExpandEnumCapability,
 		InstantAddLastColumnFlavorCapability,
 		InstantAddDropVirtualColumnFlavorCapability,
 		InstantChangeColumnDefaultFlavorCapability:

go/mysql/flavor_mysqlgr.go

@@ -247,6 +247,7 @@ func (mysqlGRFlavor) baseShowTablesWithSizes() string {
 func (mysqlGRFlavor) supportsCapability(serverVersion string, capability FlavorCapability) (bool, error) {
 	switch capability {
 	case InstantDDLFlavorCapability,
+		InstantExpandEnumCapability,
 		InstantAddLastColumnFlavorCapability,
 		InstantAddDropVirtualColumnFlavorCapability,
 		InstantChangeColumnDefaultFlavorCapability:

go/mysql/mysql56_gtid_set.go

@@ -53,9 +53,8 @@ func parseInterval(s string) (interval, error) {
 			return interval{}, vterrors.Wrapf(err, "invalid interval (%q)", s)
 		}
 		return interval{start: int64(start), end: int64(end)}, nil
-	} else {
-		return interval{start: int64(start), end: int64(start)}, nil
 	}
+	return interval{start: int64(start), end: int64(start)}, nil
 }
 
 // ParseMysql56GTIDSet is registered as a GTIDSet parser.

go/test/endtoend/onlineddl/revert/onlineddl_revert_test.go

@@ -654,7 +654,7 @@ func TestSchemaChange(t *testing.T) {
 
 	// INSTANT DDL
 	t.Run("INSTANT DDL: add column", func(t *testing.T) {
-		uuid := testOnlineDDLStatementForTable(t, "alter table stress_test add column i_instant int not null default 0", ddlStrategy+" --fast-over-revertible", "vtgate", "i_instant")
+		uuid := testOnlineDDLStatementForTable(t, "alter table stress_test add column i_instant int not null default 0", ddlStrategy+" --prefer-instant-ddl", "vtgate", "i_instant")
 		uuids = append(uuids, uuid)
 		onlineddl.CheckMigrationStatus(t, &vtParams, shards, uuid, schema.OnlineDDLStatusComplete)
 		checkTable(t, tableName, true)

go/test/endtoend/onlineddl/vrepl/onlineddl_vrepl_test.go

@@ -627,14 +627,14 @@ func TestSchemaChange(t *testing.T) {
 	})
 	t.Run("Online CREATE, vtctl, extra flags", func(t *testing.T) {
 		// the flags are meaningless to this migration. The test just validates that they don't get in the way.
-		uuid := testOnlineDDLStatement(t, onlineDDLCreateTableStatement, "vitess --fast-over-revertible --allow-zero-in-date", providedUUID, providedMigrationContext, "vtctl", "online_ddl_create_col", "", false)
+		uuid := testOnlineDDLStatement(t, onlineDDLCreateTableStatement, "vitess --prefer-instant-ddl --allow-zero-in-date", providedUUID, providedMigrationContext, "vtctl", "online_ddl_create_col", "", false)
 		onlineddl.CheckMigrationStatus(t, &vtParams, shards, uuid, schema.OnlineDDLStatusComplete)
 		onlineddl.CheckCancelMigration(t, &vtParams, shards, uuid, false)
 		onlineddl.CheckRetryMigration(t, &vtParams, shards, uuid, false)
 	})
 	t.Run("Online DROP TABLE IF EXISTS, vtgate, extra flags", func(t *testing.T) {
 		// the flags are meaningless to this migration. The test just validates that they don't get in the way.
-		uuid := testOnlineDDLStatement(t, onlineDDLDropTableIfExistsStatement, "vitess --fast-over-revertible --allow-zero-in-date", providedUUID, providedMigrationContext, "vtgate", "", "", false)
+		uuid := testOnlineDDLStatement(t, onlineDDLDropTableIfExistsStatement, "vitess --prefer-instant-ddl --allow-zero-in-date", providedUUID, providedMigrationContext, "vtgate", "", "", false)
 		onlineddl.CheckMigrationStatus(t, &vtParams, shards, uuid, schema.OnlineDDLStatusComplete)
 		onlineddl.CheckCancelMigration(t, &vtParams, shards, uuid, false)
 		onlineddl.CheckRetryMigration(t, &vtParams, shards, uuid, false)

go/vt/schema/ddl_strategy.go

@@ -36,7 +36,7 @@ const (
 	postponeLaunchFlag     = "postpone-launch"
 	postponeCompletionFlag = "postpone-completion"
 	allowConcurrentFlag    = "allow-concurrent"
-	fastOverRevertibleFlag = "fast-over-revertible"
+	preferInstantDDL       = "prefer-instant-ddl"
 	fastRangeRotationFlag  = "fast-range-rotation"
 	vreplicationTestSuite  = "vreplication-test-suite"
 )
@@ -158,9 +158,9 @@ func (setting *DDLStrategySetting) IsAllowConcurrent() bool {
 	return setting.hasFlag(allowConcurrentFlag)
 }
 
-// IsFastOverRevertibleFlag checks if strategy options include -fast-over-revertible
-func (setting *DDLStrategySetting) IsFastOverRevertibleFlag() bool {
-	return setting.hasFlag(fastOverRevertibleFlag)
+// IsPreferInstantDDL checks if strategy options include -prefer-instant-ddl
+func (setting *DDLStrategySetting) IsPreferInstantDDL() bool {
+	return setting.hasFlag(preferInstantDDL)
 }
 
 // IsFastRangeRotationFlag checks if strategy options include -fast-range-rotation
@@ -187,7 +187,7 @@ func (setting *DDLStrategySetting) RuntimeOptions() []string {
 		case isFlag(opt, postponeLaunchFlag):
 		case isFlag(opt, postponeCompletionFlag):
 		case isFlag(opt, allowConcurrentFlag):
-		case isFlag(opt, fastOverRevertibleFlag):
+		case isFlag(opt, preferInstantDDL):
 		case isFlag(opt, fastRangeRotationFlag):
 		case isFlag(opt, vreplicationTestSuite):
 		default:

go/vt/schema/ddl_strategy_test.go

@@ -132,9 +132,9 @@ func TestParseDDLStrategy(t *testing.T) {
 			isAllowConcurrent: true,
 		},
 		{
-			strategyVariable:   "vitess --fast-over-revertible",
+			strategyVariable:   "vitess --prefer-instant-ddl",
 			strategy:           DDLStrategyVitess,
-			options:            "--fast-over-revertible",
+			options:            "--prefer-instant-ddl",
 			runtimeOptions:     "",
 			fastOverRevertible: true,
 		},
@@ -156,7 +156,7 @@ func TestParseDDLStrategy(t *testing.T) {
 		assert.Equal(t, ts.isPostponeCompletion, setting.IsPostponeCompletion())
 		assert.Equal(t, ts.isPostponeLaunch, setting.IsPostponeLaunch())
 		assert.Equal(t, ts.isAllowConcurrent, setting.IsAllowConcurrent())
-		assert.Equal(t, ts.fastOverRevertible, setting.IsFastOverRevertibleFlag())
+		assert.Equal(t, ts.fastOverRevertible, setting.IsPreferInstantDDL())
 		assert.Equal(t, ts.fastRangeRotation, setting.IsFastRangeRotationFlag())
 
 		runtimeOptions := strings.Join(setting.RuntimeOptions(), " ")

go/vt/vttablet/onlineddl/analysis.go

@@ -175,6 +175,8 @@ func analyzeAddRangePartition(alterTable *sqlparser.AlterTable, createTable *sql
 	return op
 }
 
+// alterOptionAvailableViaInstantDDL chcks if the specific alter option is eligible to run via ALGORITHM=INSTANT
+// reference: https://dev.mysql.com/doc/refman/8.0/en/innodb-online-ddl-operations.html
 func alterOptionAvailableViaInstantDDL(alterOption sqlparser.AlterOption, createTable *sqlparser.CreateTable, capableOf mysql.CapableOf) (bool, error) {
 	findColumn := func(colName string) *sqlparser.ColumnDefinition {
 		if createTable == nil {
@@ -187,6 +189,17 @@ func alterOptionAvailableViaInstantDDL(alterOption sqlparser.AlterOption, create
 		}
 		return nil
 	}
+	findTableOption := func(optName string) *sqlparser.TableOption {
+		if createTable == nil {
+			return nil
+		}
+		for _, opt := range createTable.TableSpec.Options {
+			if strings.EqualFold(optName, opt.Name) {
+				return opt
+			}
+		}
+		return nil
+	}
 	isVirtualColumn := func(colName string) bool {
 		col := findColumn(colName)
 		if col == nil {
@@ -200,13 +213,36 @@ func alterOptionAvailableViaInstantDDL(alterOption sqlparser.AlterOption, create
 		}
 		return col.Type.Options.Storage == sqlparser.VirtualStorage
 	}
-	colStringWithoutDefault := func(col *sqlparser.ColumnDefinition) string {
-		colWithoutDefault := sqlparser.CloneRefOfColumnDefinition(col)
-		colWithoutDefault.Type.Options.Default = nil
-		return sqlparser.CanonicalString(colWithoutDefault)
+	colStringStrippedDown := func(col *sqlparser.ColumnDefinition, stripDefault bool, stripEnum bool) string {
+		strippedCol := sqlparser.CloneRefOfColumnDefinition(col)
+		if stripDefault {
+			strippedCol.Type.Options.Default = nil
+		}
+		if stripEnum {
+			strippedCol.Type.EnumValues = nil
+		}
+		return sqlparser.CanonicalString(strippedCol)
+	}
+	hasPrefix := func(vals []string, prefix []string) bool {
+		if len(vals) < len(prefix) {
+			return false
+		}
+		for i := range prefix {
+			if vals[i] != prefix[i] {
+				return false
+			}
+		}
+		return true
 	}
 	// Up to 8.0.26 we could only ADD COLUMN as last column
 	switch opt := alterOption.(type) {
+	case *sqlparser.ChangeColumn:
+		// We do not support INSTANT for renaming a column (ALTER TABLE ...CHANGE) because:
+		// 1. We discourage column rename
+		// 2. We do not produce CHANGE statements in declarative diff
+		// 3. The success of the operation depends on whether the column is referenced by a foreign key
+		//    in another table. Which is a bit too much to compute here.
+		return false, nil
 	case *sqlparser.AddColumns:
 		if opt.First || opt.After != nil {
 			// not a "last" column. Only supported as of 8.0.29
@@ -215,6 +251,12 @@ func alterOptionAvailableViaInstantDDL(alterOption sqlparser.AlterOption, create
 		// Adding a *last* column is supported in 8.0
 		return capableOf(mysql.InstantAddLastColumnFlavorCapability)
 	case *sqlparser.DropColumn:
+		// not supported in COMPRESSED tables
+		if opt := findTableOption("ROW_FORMAT"); opt != nil {
+			if strings.EqualFold(opt.String, "COMPRESSED") {
+				return false, nil
+			}
+		}
 		if isVirtualColumn(opt.Name.Name.String()) {
 			// supported by all 8.0 versions
 			return capableOf(mysql.InstantAddDropVirtualColumnFlavorCapability)
@@ -227,11 +269,42 @@ func alterOptionAvailableViaInstantDDL(alterOption sqlparser.AlterOption, create
 			// table and ALTER statement, and compare the columns: if they're otherwise equal,
 			// then the only change can be an addition/change/removal of DEFAULT, which
 			// is instant-table.
-			tableColDefinition := colStringWithoutDefault(col)
-			newColDefinition := colStringWithoutDefault(opt.NewColDefinition)
+			tableColDefinition := colStringStrippedDown(col, true, false)
+			newColDefinition := colStringStrippedDown(opt.NewColDefinition, true, false)
 			if tableColDefinition == newColDefinition {
 				return capableOf(mysql.InstantChangeColumnDefaultFlavorCapability)
 			}
+			// Check if:
+			// 1. this an ENUM/SET
+			// 2. and the change is to append values to the end of the list
+			// 3. and the number of added values does not increase the storage size for the enum/set
+			// 4. while still not caring about a change in the default value
+			if len(col.Type.EnumValues) > 0 && len(opt.NewColDefinition.Type.EnumValues) > 0 {
+				// both are enum or set
+				if !hasPrefix(opt.NewColDefinition.Type.EnumValues, col.Type.EnumValues) {
+					return false, nil
+				}
+				// we know the new column definition is identical to, or extends, the old definition.
+				// Now validate storage:
+				if strings.EqualFold(col.Type.Type, "enum") {
+					if len(col.Type.EnumValues) <= 255 && len(opt.NewColDefinition.Type.EnumValues) > 255 {
+						// this increases the SET storage size (1 byte for up to 8 values, 2 bytes beyond)
+						return false, nil
+					}
+				}
+				if strings.EqualFold(col.Type.Type, "set") {
+					if (len(col.Type.EnumValues)+7)/8 != (len(opt.NewColDefinition.Type.EnumValues)+7)/8 {
+						// this increases the SET storage size (1 byte for up to 8 values, 2 bytes for 8-15, etc.)
+						return false, nil
+					}
+				}
+				// Now don't care about change of default:
+				tableColDefinition := colStringStrippedDown(col, true, true)
+				newColDefinition := colStringStrippedDown(opt.NewColDefinition, true, true)
+				if tableColDefinition == newColDefinition {
+					return capableOf(mysql.InstantExpandEnumCapability)
+				}
+			}
 		}
 		return false, nil
 	default:
@@ -258,6 +331,7 @@ func AnalyzeInstantDDL(alterTable *sqlparser.AlterTable, createTable *sqlparser.
 		// no INSTANT for partitions
 		return nil, nil
 	}
+	// For the ALTER statement to qualify for ALGORITHM=INSTANT, all alter options must each qualify.
 	for _, alterOption := range alterTable.AlterOptions {
 		instantOK, err := alterOptionAvailableViaInstantDDL(alterOption, createTable, capableOf)
 		if err != nil {
@@ -303,7 +377,7 @@ func (e *Executor) analyzeSpecialAlterPlan(ctx context.Context, onlineDDL *schem
 			return op, nil
 		}
 	}
-	if onlineDDL.StrategySetting().IsFastOverRevertibleFlag() {
+	if onlineDDL.StrategySetting().IsPreferInstantDDL() {
 		op, err := AnalyzeInstantDDL(alterTable, createTable, capableOf)
 		if err != nil {
 			return nil, err

go/vt/vttablet/onlineddl/analysis_test.go

@@ -58,6 +58,7 @@ func TestAnalyzeInstantDDL(t *testing.T) {
 			instant: true,
 		},
 		{
+			// fail add mid column in older versions
 			version: "8.0.21",
 			create:  "create table t(id int, i1 int not null, primary key(id))",
 			alter:   "alter table t add column i2 int not null after id",
@@ -70,6 +71,7 @@ func TestAnalyzeInstantDDL(t *testing.T) {
 			instant: false,
 		},
 		{
+			// drop virtual column
 			version: "8.0.21",
 			create:  "create table t(id int, i1 int not null, i2 int generated always as (i1 + 1) virtual, primary key(id))",
 			alter:   "alter table t drop column i2",
@@ -82,17 +84,26 @@ func TestAnalyzeInstantDDL(t *testing.T) {
 			instant: false,
 		},
 		{
+			// add mid column
 			version: "8.0.29",
 			create:  "create table t(id int, i1 int not null, primary key(id))",
 			alter:   "alter table t add column i2 int not null after id",
 			instant: true,
 		},
 		{
+			// drop mid column
 			version: "8.0.29",
 			create:  "create table t(id int, i1 int not null, i2 int not null, primary key(id))",
 			alter:   "alter table t drop column i1",
 			instant: true,
 		},
+		{
+			// fail due to row_format=compressed
+			version: "8.0.29",
+			create:  "create table t(id int, i1 int not null, i2 int not null, primary key(id)) row_format=compressed",
+			alter:   "alter table t drop column i1",
+			instant: false,
+		},
 		{
 			version: "8.0.29",
 			create:  "create table t(id int, i1 int not null, primary key(id))",
@@ -107,24 +118,28 @@ func TestAnalyzeInstantDDL(t *testing.T) {
 		},
 		// change/remove column default
 		{
+			// set a default value
 			version: "8.0.21",
 			create:  "create table t(id int, i1 int not null, primary key(id))",
 			alter:   "alter table t modify column i1 int not null default 0",
 			instant: true,
 		},
 		{
+			// change a default value
 			version: "8.0.21",
 			create:  "create table t(id int, i1 int not null, primary key(id))",
 			alter:   "alter table t modify column i1 int not null default 3",
 			instant: true,
 		},
 		{
+			// change default value to null
 			version: "8.0.21",
 			create:  "create table t(id int, i1 int not null, primary key(id))",
 			alter:   "alter table t modify column i1 int default null",
 			instant: false,
 		},
 		{
+			// fail because on top of changing the default value, the datatype is changed, too
 			version: "8.0.21",
 			create:  "create table t(id int, i1 int not null, primary key(id))",
 			alter:   "alter table t modify column i1 bigint not null default 3",
@@ -142,6 +157,56 @@ func TestAnalyzeInstantDDL(t *testing.T) {
 			alter:   "alter table t modify column i1 int default null",
 			instant: true,
 		},
+		// enum/set:
+		{
+			// enum same, with changed default
+			version: "8.0.21",
+			create:  "create table t(id int, c1 enum('a', 'b', 'c'), primary key(id))",
+			alter:   "alter table t modify column c1 enum('a', 'b', 'c') default 'b'",
+			instant: true,
+		},
+		{
+			// enum append
+			version: "8.0.21",
+			create:  "create table t(id int, c1 enum('a', 'b', 'c'), primary key(id))",
+			alter:   "alter table t modify column c1 enum('a', 'b', 'c', 'd')",
+			instant: true,
+		},
+		{
+			// enum append with changed default
+			version: "8.0.21",
+			create:  "create table t(id int, c1 enum('a', 'b', 'c') default 'a', primary key(id))",
+			alter:   "alter table t modify column c1 enum('a', 'b', 'c', 'd') default 'd'",
+			instant: true,
+		},
+		{
+			// fail insert in middle
+			version: "8.0.21",
+			create:  "create table t(id int, c1 enum('a', 'b', 'c'), primary key(id))",
+			alter:   "alter table t modify column c1 enum('a', 'b', 'x', 'c')",
+			instant: false,
+		},
+		{
+			// fail change
+			version: "8.0.21",
+			create:  "create table t(id int, c1 enum('a', 'b', 'c'), primary key(id))",
+			alter:   "alter table t modify column c1 enum('a', 'x', 'c')",
+			instant: false,
+		},
+		{
+			// set append
+			version: "8.0.21",
+			create:  "create table t(id int, c1 set('a', 'b', 'c'), primary key(id))",
+			alter:   "alter table t modify column c1 set('a', 'b', 'c', 'd')",
+			instant: true,
+		},
+		{
+			// fail set append when over threshold (increase from 8 to 9 values => storage goes from 1 byte to 2 bytes)
+			version: "8.0.21",
+			create:  "create table t(id int, c1 set('a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'), primary key(id))",
+			alter:   "alter table t modify column c1 set('a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i')",
+			instant: false,
+		},
 	}
 	for _, tc := range tt {
 		name := tc.version + " " + tc.create