schemadiff: more INSTANT algorithm considerations (#16678)

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
vitessio · Aug 29, 2024 · d087b63 · d087b63
1 parent e785c32
commit d087b63
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 1 deletion.
diff --git a/go/vt/schemadiff/capability.go b/go/vt/schemadiff/capability.go
@@ -151,6 +151,13 @@ func alterOptionCapableOfInstantDDL(alterOption sqlparser.AlterOption, createTab
 				// Expression default values are not supported
 				return false, nil
 			}
+			if strings.EqualFold(column.Type.Type, "datetime") {
+				e := &ColumnDefinitionEntity{ColumnDefinition: column}
+				if !e.IsNullable() && !e.HasDefault() {
+					// DATETIME columns must have a default value
+					return false, nil
+				}
+			}
 		}
 		if opt.First || opt.After != nil {
 			// not a "last" column. Only supported as of 8.0.29
@@ -185,6 +192,9 @@ func alterOptionCapableOfInstantDDL(alterOption sqlparser.AlterOption, createTab
 		}
 		return capableOf(capabilities.InstantAddDropColumnFlavorCapability)
 	case *sqlparser.ChangeColumn:
+		if opt.First || opt.After != nil {
+			return false, nil
+		}
 		// 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
@@ -198,6 +208,9 @@ func alterOptionCapableOfInstantDDL(alterOption sqlparser.AlterOption, createTab
 		}
 		return false, nil
 	case *sqlparser.ModifyColumn:
+		if opt.First || opt.After != nil {
+			return false, nil
+		}
 		if col := findColumn(opt.NewColDefinition.Name.String()); col != nil {
 			return changeModifyColumnCapableOfInstantDDL(col, opt.NewColDefinition)
 		}

diff --git a/go/vt/schemadiff/capability_test.go b/go/vt/schemadiff/capability_test.go
@@ -133,6 +133,24 @@ func TestAlterTableCapableOfInstantDDL(t *testing.T) {
 			alter:                     "alter table t add column i2 int generated always as (i1 + 1) virtual",
 			expectCapableOfInstantDDL: true,
 		},
+		{
+			name:                      "add nullable datetime column",
+			create:                    "create table t(id int, i1 int not null, primary key(id))",
+			alter:                     "alter table t add column dt datetime(3)",
+			expectCapableOfInstantDDL: true,
+		},
+		{
+			name:                      "add default null datetime column",
+			create:                    "create table t(id int, i1 int not null, primary key(id))",
+			alter:                     "alter table t add column dt datetime(3) default null",
+			expectCapableOfInstantDDL: true,
+		},
+		{
+			name:                      "add datetime column without default",
+			create:                    "create table t(id int, i1 int not null, primary key(id))",
+			alter:                     "alter table t add column dt datetime(3) not null",
+			expectCapableOfInstantDDL: false,
+		},
 		{
 			name:                      "add stored column",
 			create:                    "create table t(id int, i1 int not null, primary key(id))",
@@ -225,11 +243,23 @@ func TestAlterTableCapableOfInstantDDL(t *testing.T) {
 			expectCapableOfInstantDDL: false,
 		},
 		{
-			name:                      "set column dfault value to null",
+			name:                      "set column default value to null",
 			create:                    "create table t(id int, i1 int, primary key(id))",
 			alter:                     "alter table t modify column i1 int default null",
 			expectCapableOfInstantDDL: true,
 		},
+		{
+			name:                      "rearrange column after",
+			create:                    "create table t(id int, i1 int, i2 int, primary key(id))",
+			alter:                     "alter table t modify column i1 int after i2",
+			expectCapableOfInstantDDL: false,
+		},
+		{
+			name:                      "rearrange column first",
+			create:                    "create table t(id int, i1 int, i2 int, primary key(id))",
+			alter:                     "alter table t modify column i1 int first",
+			expectCapableOfInstantDDL: false,
+		},
 		// enum/set:
 		{
 			name:                      "change enum default value",
@@ -291,6 +321,18 @@ func TestAlterTableCapableOfInstantDDL(t *testing.T) {
 			alter:                     "alter table t1 change column i1 i2 int visible",
 			expectCapableOfInstantDDL: false,
 		},
+		{
+			name:                      "change column, first",
+			create:                    "create table t1 (id int, i1 int, i2 int)",
+			alter:                     "alter table t1 change column i1 i1 int first",
+			expectCapableOfInstantDDL: false,
+		},
+		{
+			name:                      "change column, after",
+			create:                    "create table t1 (id int, i1 int, i2 int)",
+			alter:                     "alter table t1 change column i1 i1 int after i2",
+			expectCapableOfInstantDDL: false,
+		},
 		{
 			name:                      "make a column invisible via SET",
 			create:                    "create table t1 (id int, i1 int)",