remove RangeRotationCombinedStatements. Make RangeRotationDistinctSta…

…tements combine DROP PARTITION statements Signed-off-by: Shlomi Noach <[email protected]>
planetscale · Apr 8, 2024 · 0943790 · 0943790
1 parent 0f4f100
commit 0943790
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 62 deletions.
diff --git a/go/vt/schemadiff/table.go b/go/vt/schemadiff/table.go
@@ -1244,29 +1244,15 @@ func (c *CreateTableEntity) isRangePartitionsRotation(
 	var partitionSpecs []*sqlparser.PartitionSpec
 	// Dropped partitions:
 	if len(droppedPartitions1) > 0 {
-		switch hints.RangeRotationStrategy {
-		case RangeRotationCombinedStatements:
-			// A single DROP PARTITION with multiple partition names
-			partitionSpec := &sqlparser.PartitionSpec{
-				Action: sqlparser.DropAction,
-			}
-			for _, p := range droppedPartitions1 {
-				partitionSpec.Names = append(partitionSpec.Names, p.Name)
-			}
-			partitionSpecs = append(partitionSpecs, partitionSpec)
-		default:
-			// Multiple DROP PARTITION each with a single partition name
-			for _, p := range droppedPartitions1 {
-				partitionSpec := &sqlparser.PartitionSpec{
-					Action: sqlparser.DropAction,
-					Names:  []sqlparser.IdentifierCI{p.Name},
-				}
-				partitionSpecs = append(partitionSpecs, partitionSpec)
-			}
+		// A single DROP PARTITION clause can specify multiple partition names
+		partitionSpec := &sqlparser.PartitionSpec{
+			Action: sqlparser.DropAction,
 		}
 		for _, p := range droppedPartitions1 {
+			partitionSpec.Names = append(partitionSpec.Names, p.Name)
 			annotations.MarkRemoved(sqlparser.CanonicalString(p))
 		}
+		partitionSpecs = append(partitionSpecs, partitionSpec)
 	}
 	// Added partitions:
 	for _, p := range addedPartitions2 {
@@ -1324,8 +1310,7 @@ func (c *CreateTableEntity) diffPartitions(alterTable *sqlparser.AlterTable,
 			switch hints.RangeRotationStrategy {
 			case RangeRotationIgnore:
 				return nil, nil
-			case RangeRotationCombinedStatements,
-				RangeRotationDistinctStatements:
+			case RangeRotationDistinctStatements:
 				return partitionSpecs, nil
 			case RangeRotationFullSpec:
 				// proceed to return a full rebuild

diff --git a/go/vt/schemadiff/table_test.go b/go/vt/schemadiff/table_test.go
@@ -1332,17 +1332,6 @@ func TestCreateTableDiff(t *testing.T) {
 				"-(PARTITION `p1` VALUES LESS THAN (10),",
 			},
 		},
-		{
-			name:     "change partitioning range: statements, drop, combined",
-			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))",
-			rotation: RangeRotationCombinedStatements, // combining just 1 statement is trivially identical to distinct statements output
-			diff:     "alter table t1 drop partition p1",
-			cdiff:    "ALTER TABLE `t1` DROP PARTITION `p1`",
-			textdiffs: []string{
-				"-(PARTITION `p1` VALUES LESS THAN (10),",
-			},
-		},
 		{
 			name:     "change partitioning range: statements, add",
 			from:     "create table t1 (id int primary key) partition by range (id) (partition p1 values less than (10), partition p2 values less than (20))",
@@ -1354,25 +1343,13 @@ func TestCreateTableDiff(t *testing.T) {
 				"+ PARTITION `p3` VALUES LESS THAN (30)",
 			},
 		},
-		{
-			name:     "change partitioning range: statements, multiple drops, combined",
-			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 p3 values less than (30))",
-			rotation: RangeRotationCombinedStatements,
-			diffs:    []string{"alter table t1 drop partition p1, p2"},
-			cdiffs:   []string{"ALTER TABLE `t1` DROP PARTITION `p1`, `p2`"},
-			textdiffs: []string{
-				"-(PARTITION `p1` VALUES LESS THAN (10),",
-				"- PARTITION `p2` VALUES LESS THAN (20),",
-			},
-		},
 		{
 			name:     "change partitioning range: statements, multiple drops, distinct",
 			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 p3 values less than (30))",
 			rotation: RangeRotationDistinctStatements,
-			diffs:    []string{"alter table t1 drop partition p1", "alter table t1 drop partition p2"},
-			cdiffs:   []string{"ALTER TABLE `t1` DROP PARTITION `p1`", "ALTER TABLE `t1` DROP PARTITION `p2`"},
+			diffs:    []string{"alter table t1 drop partition p1, p2"},
+			cdiffs:   []string{"ALTER TABLE `t1` DROP PARTITION `p1`, `p2`"},
 			textdiffs: []string{
 				"-(PARTITION `p1` VALUES LESS THAN (10),",
 				"- PARTITION `p2` VALUES LESS THAN (20),",
@@ -1390,18 +1367,6 @@ func TestCreateTableDiff(t *testing.T) {
 				"+ PARTITION `p3` VALUES LESS THAN (30)",
 			},
 		},
-		{
-			name:     "change partitioning range: statements, multiple adds, combined",
-			from:     "create table t1 (id int primary key) partition by range (id) (partition p1 values less than (10))",
-			to:       "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))",
-			rotation: RangeRotationCombinedStatements, // ADD PARTITION syntax only supports a single partition so _combining_ is not different than RangeRotationDistinctStatements
-			diffs:    []string{"alter table t1 add partition (partition p2 values less than (20))", "alter table t1 add partition (partition p3 values less than (30))"},
-			cdiffs:   []string{"ALTER TABLE `t1` ADD PARTITION (PARTITION `p2` VALUES LESS THAN (20))", "ALTER TABLE `t1` ADD PARTITION (PARTITION `p3` VALUES LESS THAN (30))"},
-			textdiffs: []string{
-				"+ PARTITION `p2` VALUES LESS THAN (20),",
-				"+ PARTITION `p3` VALUES LESS THAN (30)",
-			},
-		},
 		{
 			name:     "change partitioning range: statements, multiple, assorted",
 			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))",
@@ -1427,8 +1392,8 @@ func TestCreateTableDiff(t *testing.T) {
 			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,
-			diffs:    []string{"alter table t1 add column i int", "alter table t1 drop partition p1", "alter table t1 drop partition p2"},
-			cdiffs:   []string{"ALTER TABLE `t1` ADD COLUMN `i` int", "ALTER TABLE `t1` DROP PARTITION `p1`", "ALTER TABLE `t1` DROP PARTITION `p2`"},
+			diffs:    []string{"alter table t1 add column i int", "alter table t1 drop partition p1, p2"},
+			cdiffs:   []string{"ALTER TABLE `t1` ADD COLUMN `i` int", "ALTER TABLE `t1` DROP PARTITION `p1`, `p2`"},
 			textdiffs: []string{
 				"+	`i` int",
 				"-(PARTITION `p1` VALUES LESS THAN (10),",
@@ -2071,7 +2036,7 @@ func TestCreateTableDiff(t *testing.T) {
 				if len(ts.diffs) > 0 {
 
 					allSubsequentDiffs := AllSubsequent(alter)
-					require.Equal(t, len(ts.diffs), len(allSubsequentDiffs))
+					require.Equal(t, len(ts.diffs), len(allSubsequentDiffs), alter.CanonicalStatementString())
 					require.Equal(t, len(ts.cdiffs), len(allSubsequentDiffs))
 					for i := range ts.diffs {
 						assert.Equal(t, ts.diffs[i], allSubsequentDiffs[i].StatementString())

diff --git a/go/vt/schemadiff/types.go b/go/vt/schemadiff/types.go
@@ -81,7 +81,6 @@ const (
 
 const (
 	RangeRotationFullSpec = iota
-	RangeRotationCombinedStatements
 	RangeRotationDistinctStatements
 	RangeRotationIgnore
 )