schemadiff: SubsequentDiffStrategy: allow/reject multiple changes…

… on same entity (vitessio#15675)

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

go/vt/schemadiff/errors.go

@@ -453,3 +453,18 @@ type UnknownColumnCollationCharsetError struct {
 func (e *UnknownColumnCollationCharsetError) Error() string {
 	return fmt.Sprintf("unable to determine charset for column %s with collation %q", sqlescape.EscapeID(e.Column), e.Collation)
 }
+
+type SubsequentDiffRejectedError struct {
+	Table string
+	Diffs []EntityDiff
+}
+
+func (e *SubsequentDiffRejectedError) Error() string {
+	var b strings.Builder
+	b.WriteString(fmt.Sprintf("multiple changes not allowed on table %s. Found:", sqlescape.EscapeID(e.Table)))
+	for _, d := range e.Diffs {
+		b.WriteString("\n")
+		b.WriteString(d.CanonicalStatementString())
+	}
+	return b.String()
+}

go/vt/schemadiff/table.go

@@ -946,6 +946,12 @@ func (c *CreateTableEntity) TableDiff(other *CreateTableEntity, hints *DiffHints
 	}
 	sortAlterOptions(parentAlterTableEntityDiff)
 
+	if hints.SubsequentDiffStrategy == SubsequentDiffStrategyReject {
+		if allSubsequent := AllSubsequent(parentAlterTableEntityDiff); len(allSubsequent) > 1 {
+			return nil, &SubsequentDiffRejectedError{Table: c.Name(), Diffs: allSubsequent}
+		}
+	}
+
 	return parentAlterTableEntityDiff, nil
 }
 

go/vt/schemadiff/table_test.go

@@ -46,6 +46,7 @@ func TestCreateTableDiff(t *testing.T) {
 		charset     int
 		algorithm   int
 		enumreorder int
+		subsequent  int
 		textdiffs   []string
 	}{
 		{
@@ -803,6 +804,13 @@ func TestCreateTableDiff(t *testing.T) {
 				"+	FULLTEXT KEY `name2_ft` (`name2`)",
 			},
 		},
+		{
+			name:       "add two fulltext keys, distinct statements, reject",
+			from:       "create table t1 (id int primary key, name1 tinytext not null, name2 tinytext not null)",
+			to:         "create table t1 (id int primary key, name1 tinytext not null, name2 tinytext not null, fulltext key name1_ft(name1), fulltext key name2_ft(name2))",
+			subsequent: SubsequentDiffStrategyReject,
+			errorMsg:   (&SubsequentDiffRejectedError{Table: "t1"}).Error(),
+		},
 		{
 			name:     "add two fulltext keys, unify statements",
 			from:     "create table t1 (id int primary key, name1 tinytext not null, name2 tinytext not null)",
@@ -815,6 +823,19 @@ func TestCreateTableDiff(t *testing.T) {
 				"+	FULLTEXT KEY `name2_ft` (`name2`)",
 			},
 		},
+		{
+			name:       "add two fulltext keys, unify statements, no reject",
+			from:       "create table t1 (id int primary key, name1 tinytext not null, name2 tinytext not null)",
+			to:         "create table t1 (id int primary key, name1 tinytext not null, name2 tinytext not null, fulltext key name1_ft(name1), fulltext key name2_ft(name2))",
+			fulltext:   FullTextKeyUnifyStatements,
+			subsequent: SubsequentDiffStrategyReject,
+			diff:       "alter table t1 add fulltext key name1_ft (name1), add fulltext key name2_ft (name2)",
+			cdiff:      "ALTER TABLE `t1` ADD FULLTEXT KEY `name1_ft` (`name1`), ADD FULLTEXT KEY `name2_ft` (`name2`)",
+			textdiffs: []string{
+				"+	FULLTEXT KEY `name1_ft` (`name1`)",
+				"+	FULLTEXT KEY `name2_ft` (`name2`)",
+			},
+		},
 		{
 			name: "no fulltext diff",
 			from: "create table t1 (id int primary key, name tinytext not null, fulltext key name_ft(name) with parser ngram)",
@@ -1358,6 +1379,14 @@ func TestCreateTableDiff(t *testing.T) {
 				"+ PARTITION `p4` VALUES LESS THAN (40)",
 			},
 		},
+		{
+			name:       "change partitioning range: statements, multiple, reject",
+			from:       "create table t1 (id int primary key) partition by range (id) (partition p1 values less than (10), partition p2 values less than (20), partition p3 values less than (30))",
+			to:         "create table t1 (id int primary key) partition by range (id) (partition p2 values less than (20), partition p3 values less than (30), partition p4 values less than (40))",
+			rotation:   RangeRotationDistinctStatements,
+			subsequent: SubsequentDiffStrategyReject,
+			errorMsg:   (&SubsequentDiffRejectedError{Table: "t1"}).Error(),
+		},
 		{
 			name:     "change partitioning range: mixed with nonpartition changes",
 			from:     "create table t1 (id int primary key) partition by range (id) (partition p1 values less than (10), partition p2 values less than (20), partition p3 values less than (30))",
@@ -1371,6 +1400,14 @@ func TestCreateTableDiff(t *testing.T) {
 				"- PARTITION `p2` VALUES LESS THAN (20),",
 			},
 		},
+		{
+			name:       "change partitioning range: mixed with nonpartition changes, reject",
+			from:       "create table t1 (id int primary key) partition by range (id) (partition p1 values less than (10), partition p2 values less than (20), partition p3 values less than (30))",
+			to:         "create table t1 (id int primary key, i int) partition by range (id) (partition p3 values less than (30))",
+			rotation:   RangeRotationDistinctStatements,
+			subsequent: SubsequentDiffStrategyReject,
+			errorMsg:   (&SubsequentDiffRejectedError{Table: "t1"}).Error(),
+		},
 		{
 			name:     "change partitioning range: single partition change, mixed with nonpartition changes",
 			from:     "create table t1 (id int primary key) partition by range (id) (partition p1 values less than (10), partition p2 values less than (20))",
@@ -1953,6 +1990,7 @@ func TestCreateTableDiff(t *testing.T) {
 			hints.TableCharsetCollateStrategy = ts.charset
 			hints.AlterTableAlgorithmStrategy = ts.algorithm
 			hints.EnumReorderStrategy = ts.enumreorder
+			hints.SubsequentDiffStrategy = ts.subsequent
 			alter, err := c.Diff(other, &hints)
 
 			require.Equal(t, len(ts.diffs), len(ts.cdiffs))
@@ -2367,6 +2405,7 @@ func TestValidate(t *testing.T) {
 			alter:     "alter table t add column i int",
 			expectErr: &ApplyDuplicatePartitionError{Table: "t1", Partition: "p2"},
 		},
+		// More columns and indexes
 		{
 			name:  "change to visible with alter column",
 			from:  "create table t (id int, i int invisible, primary key (id))",

go/vt/schemadiff/types.go

@@ -134,6 +134,11 @@ const (
 	ForeignKeyCheckStrategyIgnore
 )
 
+const (
+	SubsequentDiffStrategyAllow int = iota
+	SubsequentDiffStrategyReject
+)
+
 // DiffHints is an assortment of rules for diffing entities
 type DiffHints struct {
 	StrictIndexOrdering         bool
@@ -148,6 +153,7 @@ type DiffHints struct {
 	AlterTableAlgorithmStrategy int
 	EnumReorderStrategy         int
 	ForeignKeyCheckStrategy     int
+	SubsequentDiffStrategy      int
 }
 
 func EmptyDiffHints() *DiffHints {