schemadiff: atomic diffs for range partition DROP PARTITION state…

…ment (#15843)

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

go/vt/schemadiff/diff.go

@@ -24,6 +24,36 @@ func AllSubsequent(diff EntityDiff) (diffs []EntityDiff) {
 	return diffs
 }
 
+// AtomicDiffs attempts to break a given diff statement into its smallest components.
+// This isn't necessarily the _correct_ thing to do, as MySQL goes, but it assists in
+// identifying the distinct changes that are being made.
+// Currently, the only implementation is to break up `ALTER TABLE ... DROP PARTITION` statements.
+func AtomicDiffs(diff EntityDiff) []EntityDiff {
+	if diff == nil || diff.IsEmpty() {
+		return nil
+	}
+	trivial := func() []EntityDiff {
+		return []EntityDiff{diff}
+	}
+	switch diff := diff.(type) {
+	case *AlterTableEntityDiff:
+		alterTable := diff.alterTable
+		// Examine the scenario where we have e.g. `ALTER TABLE ... DROP PARTITION p1, p2, p3`
+		// and explode it into separate diffs
+		if alterTable.PartitionSpec != nil && alterTable.PartitionSpec.Action == sqlparser.DropAction && len(alterTable.PartitionSpec.Names) > 1 {
+			var distinctDiffs []EntityDiff
+			for i := range alterTable.PartitionSpec.Names {
+				clone := diff.Clone()
+				cloneAlterTableEntityDiff := clone.(*AlterTableEntityDiff)
+				cloneAlterTableEntityDiff.alterTable.PartitionSpec.Names = cloneAlterTableEntityDiff.alterTable.PartitionSpec.Names[i : i+1]
+				distinctDiffs = append(distinctDiffs, clone)
+			}
+			return distinctDiffs
+		}
+	}
+	return trivial()
+}
+
 // DiffCreateTablesQueries compares two `CREATE TABLE ...` queries (in string form) and returns the diff from table1 to table2.
 // Either or both of the queries can be empty. Based on this, the diff could be
 // nil, CreateTable, DropTable or AlterTable

go/vt/schemadiff/diff_test.go

@@ -569,6 +569,11 @@ func TestDiffViews(t *testing.T) {
 				diff := dq.StatementString()
 				assert.Equal(t, ts.diff, diff)
 			}
+			if d != nil && !d.IsEmpty() {
+				// Validate Clone() works
+				clone := d.Clone()
+				assert.Equal(t, d.Statement(), clone.Statement())
+			}
 		})
 	}
 }
@@ -1061,6 +1066,22 @@ func TestDiffSchemas(t *testing.T) {
 					_, err := env.Parser().ParseStrictDDL(s)
 					assert.NoError(t, err)
 				}
+				// Validate Clone() works
+				for _, d := range diffs {
+					if d.IsEmpty() {
+						continue
+					}
+					dFrom, dTo := d.Entities()
+					clone := d.Clone()
+					clonedFrom, clonedTo := clone.Entities()
+					assert.Equal(t, d.CanonicalStatementString(), clone.CanonicalStatementString())
+					if dFrom != nil {
+						assert.Equal(t, dFrom.Name(), clonedFrom.Name())
+					}
+					if dTo != nil {
+						assert.Equal(t, dTo.Name(), clonedTo.Name())
+					}
+				}
 
 				if ts.annotated != nil {
 					// Optional test for assorted scenarios.

go/vt/schemadiff/table.go

@@ -139,6 +139,29 @@ func (d *AlterTableEntityDiff) InstantDDLCapability() InstantDDLCapability {
 	return d.instantDDLCapability
 }
 
+// Clone implements EntityDiff
+func (d *AlterTableEntityDiff) Clone() EntityDiff {
+	if d == nil {
+		return nil
+	}
+	ann := *d.annotations
+	clone := &AlterTableEntityDiff{
+		alterTable:           sqlparser.CloneRefOfAlterTable(d.alterTable),
+		instantDDLCapability: d.instantDDLCapability,
+		annotations:          &ann,
+	}
+	if d.from != nil {
+		clone.from = d.from.Clone().(*CreateTableEntity)
+	}
+	if d.to != nil {
+		clone.to = d.to.Clone().(*CreateTableEntity)
+	}
+	if d.subsequentDiff != nil {
+		clone.subsequentDiff = d.subsequentDiff.Clone().(*AlterTableEntityDiff)
+	}
+	return clone
+}
+
 type CreateTableEntityDiff struct {
 	to          *CreateTableEntity
 	createTable *sqlparser.CreateTable
@@ -216,6 +239,20 @@ func (d *CreateTableEntityDiff) InstantDDLCapability() InstantDDLCapability {
 	return InstantDDLCapabilityIrrelevant
 }
 
+// Clone implements EntityDiff
+func (d *CreateTableEntityDiff) Clone() EntityDiff {
+	if d == nil {
+		return nil
+	}
+	clone := &CreateTableEntityDiff{
+		createTable: sqlparser.CloneRefOfCreateTable(d.createTable),
+	}
+	if d.to != nil {
+		clone.to = d.to.Clone().(*CreateTableEntity)
+	}
+	return clone
+}
+
 type DropTableEntityDiff struct {
 	from      *CreateTableEntity
 	dropTable *sqlparser.DropTable
@@ -293,6 +330,20 @@ func (d *DropTableEntityDiff) InstantDDLCapability() InstantDDLCapability {
 	return InstantDDLCapabilityIrrelevant
 }
 
+// Clone implements EntityDiff
+func (d *DropTableEntityDiff) Clone() EntityDiff {
+	if d == nil {
+		return nil
+	}
+	clone := &DropTableEntityDiff{
+		dropTable: sqlparser.CloneRefOfDropTable(d.dropTable),
+	}
+	if d.from != nil {
+		clone.from = d.from.Clone().(*CreateTableEntity)
+	}
+	return clone
+}
+
 type RenameTableEntityDiff struct {
 	from        *CreateTableEntity
 	to          *CreateTableEntity
@@ -371,6 +422,23 @@ func (d *RenameTableEntityDiff) InstantDDLCapability() InstantDDLCapability {
 	return InstantDDLCapabilityIrrelevant
 }
 
+// Clone implements EntityDiff
+func (d *RenameTableEntityDiff) Clone() EntityDiff {
+	if d == nil {
+		return nil
+	}
+	clone := &RenameTableEntityDiff{
+		renameTable: sqlparser.CloneRefOfRenameTable(d.renameTable),
+	}
+	if d.from != nil {
+		clone.from = d.from.Clone().(*CreateTableEntity)
+	}
+	if d.to != nil {
+		clone.to = d.to.Clone().(*CreateTableEntity)
+	}
+	return clone
+}
+
 // CreateTableEntity stands for a TABLE construct. It contains the table's CREATE statement.
 type CreateTableEntity struct {
 	*sqlparser.CreateTable

go/vt/schemadiff/table_test.go

@@ -48,6 +48,7 @@ func TestCreateTableDiff(t *testing.T) {
 		enumreorder int
 		subsequent  int
 		textdiffs   []string
+		atomicdiffs []string
 	}{
 		{
 			name: "identical",
@@ -1365,6 +1366,10 @@ func TestCreateTableDiff(t *testing.T) {
 				"-(PARTITION `p1` VALUES LESS THAN (10),",
 				"- PARTITION `p2` VALUES LESS THAN (20),",
 			},
+			atomicdiffs: []string{
+				"ALTER TABLE `t1` DROP PARTITION `p1`",
+				"ALTER TABLE `t1` DROP PARTITION `p2`",
+			},
 		},
 		{
 			name:     "change partitioning range: statements, multiple adds",
@@ -1503,6 +1508,10 @@ func TestCreateTableDiff(t *testing.T) {
 				"+ PARTITION `p3` VALUES LESS THAN (35),",
 				"+ PARTITION `p4` VALUES LESS THAN (40))",
 			},
+			atomicdiffs: []string{
+				"ALTER TABLE `t1` DROP PARTITION `p1`",
+				"ALTER TABLE `t1` DROP PARTITION `p3`",
+			},
 		},
 		{
 			name:     "change partitioning range: complex rotate 2, ignore",
@@ -1523,6 +1532,10 @@ func TestCreateTableDiff(t *testing.T) {
 				"+ PARTITION `pX` VALUES LESS THAN (30),",
 				"+ PARTITION `p4` VALUES LESS THAN (40))",
 			},
+			atomicdiffs: []string{
+				"ALTER TABLE `t1` DROP PARTITION `p1`",
+				"ALTER TABLE `t1` DROP PARTITION `p3`",
+			},
 		},
 		{
 			name:     "change partitioning range: not a rotation",
@@ -2077,6 +2090,7 @@ func TestCreateTableDiff(t *testing.T) {
 					t.Logf("other: %v", sqlparser.CanonicalString(other.CreateTable))
 				}
 				assert.Empty(t, ts.textdiffs)
+				assert.Empty(t, AtomicDiffs(alter))
 				return
 			}
 
@@ -2130,6 +2144,18 @@ func TestCreateTableDiff(t *testing.T) {
 						applied.Create().CanonicalStatementString(),
 					)
 				}
+				// Validate atomic diffs
+				atomicDiffs := AtomicDiffs(alter)
+				if len(ts.atomicdiffs) > 0 {
+					assert.Equal(t, len(ts.atomicdiffs), len(atomicDiffs), "%+v", atomicDiffs)
+					for i := range ts.atomicdiffs {
+						assert.Equal(t, ts.atomicdiffs[i], atomicDiffs[i].CanonicalStatementString())
+					}
+				} else {
+					assert.Equal(t, 1, len(atomicDiffs))
+					assert.Equal(t, alter.CanonicalStatementString(), atomicDiffs[0].CanonicalStatementString())
+				}
+
 				{ // Validate annotations
 					alterEntityDiff, ok := alter.(*AlterTableEntityDiff)
 					require.True(t, ok)

go/vt/schemadiff/types.go

@@ -69,7 +69,8 @@ type EntityDiff interface {
 	SetSubsequentDiff(EntityDiff)
 	// InstantDDLCapability returns the ability of this diff to run with ALGORITHM=INSTANT
 	InstantDDLCapability() InstantDDLCapability
-
+	// Clone returns a deep copy of the entity diff, and of all referenced entities.
+	Clone() EntityDiff
 	Annotated() (from *TextualAnnotations, to *TextualAnnotations, unified *TextualAnnotations)
 }
 

go/vt/schemadiff/view.go

@@ -100,6 +100,23 @@ func (d *AlterViewEntityDiff) InstantDDLCapability() InstantDDLCapability {
 	return InstantDDLCapabilityIrrelevant
 }
 
+// Clone implements EntityDiff
+func (d *AlterViewEntityDiff) Clone() EntityDiff {
+	if d == nil {
+		return nil
+	}
+	clone := &AlterViewEntityDiff{
+		alterView: sqlparser.CloneRefOfAlterView(d.alterView),
+	}
+	if d.from != nil {
+		clone.from = d.from.Clone().(*CreateViewEntity)
+	}
+	if d.to != nil {
+		clone.to = d.to.Clone().(*CreateViewEntity)
+	}
+	return clone
+}
+
 type CreateViewEntityDiff struct {
 	createView *sqlparser.CreateView
 
@@ -177,6 +194,16 @@ func (d *CreateViewEntityDiff) InstantDDLCapability() InstantDDLCapability {
 	return InstantDDLCapabilityIrrelevant
 }
 
+// Clone implements EntityDiff
+func (d *CreateViewEntityDiff) Clone() EntityDiff {
+	if d == nil {
+		return nil
+	}
+	return &CreateViewEntityDiff{
+		createView: sqlparser.CloneRefOfCreateView(d.createView),
+	}
+}
+
 type DropViewEntityDiff struct {
 	from     *CreateViewEntity
 	dropView *sqlparser.DropView
@@ -254,6 +281,20 @@ func (d *DropViewEntityDiff) InstantDDLCapability() InstantDDLCapability {
 	return InstantDDLCapabilityIrrelevant
 }
 
+// Clone implements EntityDiff
+func (d *DropViewEntityDiff) Clone() EntityDiff {
+	if d == nil {
+		return nil
+	}
+	clone := &DropViewEntityDiff{
+		dropView: sqlparser.CloneRefOfDropView(d.dropView),
+	}
+	if d.from != nil {
+		clone.from = d.from.Clone().(*CreateViewEntity)
+	}
+	return clone
+}
+
 // CreateViewEntity stands for a VIEW construct. It contains the view's CREATE statement.
 type CreateViewEntity struct {
 	*sqlparser.CreateView

go/vt/schemadiff/view_test.go

@@ -196,6 +196,15 @@ func TestCreateViewDiff(t *testing.T) {
 						require.NoError(t, err)
 						assert.True(t, appliedDiff.IsEmpty(), "expected empty diff, found changes: %v", appliedDiff.CanonicalStatementString())
 					}
+					// Validate Clone() works
+					{
+						clone := alter.Clone()
+						alterClone, ok := clone.(*AlterViewEntityDiff)
+						require.True(t, ok)
+						assert.Equal(t, eFrom.Create().CanonicalStatementString(), alterClone.from.Create().CanonicalStatementString())
+						alterClone.from.CreateView.ViewName.Name = sqlparser.NewIdentifierCS("something_else")
+						assert.NotEqual(t, eFrom.Create().CanonicalStatementString(), alterClone.from.Create().CanonicalStatementString())
+					}
 				}
 				{
 					cdiff := alter.CanonicalStatementString()