Merge pull request #278 from cashapp/mtocker-add-unsupported-ddl

Improve support for partitioning DDL

pkg/migration/runner.go

@@ -555,10 +555,17 @@ func (r *Runner) createNewTable(ctx context.Context) error {
 
 // alterNewTable applies the ALTER to the new table.
 // It has been pre-checked it is not a rename, or modifying the PRIMARY KEY.
+// We first attempt to do this using ALGORITHM=COPY so we don't burn
+// an INSTANT version. But surprisingly this is not supported for all DDLs (issue #277)
 func (r *Runner) alterNewTable(ctx context.Context) error {
 	if err := dbconn.Exec(ctx, r.db, "ALTER TABLE %n.%n "+utils.TrimAlter(r.migration.Alter)+", ALGORITHM=COPY",
 		r.newTable.SchemaName, r.newTable.TableName); err != nil {
-		return err
+		// Retry without the ALGORITHM=COPY. If there is a second error, then the DDL itself
+		// is not supported. It could be a syntax error, in which case we return the second error,
+		// which will probably be easier to read because it is unaltered.
+		if err := dbconn.Exec(ctx, r.db, "ALTER TABLE %n.%n "+r.migration.Alter, r.newTable.SchemaName, r.newTable.TableName); err != nil {
+			return err
+		}
 	}
 	// Call GetInfo on the table again, since the columns
 	// might have changed and this will affect the row copiers intersect func.

pkg/migration/runner_test.go

@@ -48,6 +48,31 @@ func TestVarcharNonBinaryComparable(t *testing.T) {
 	assert.NoError(t, m.Close())
 }
 
+// TestPartitioningSyntax tests that ALTERs that don't support ALGORITHM assertion
+// are still supported. From https://github.com/cashapp/spirit/issues/277
+func TestPartitioningSyntax(t *testing.T) {
+	testutils.RunSQL(t, `DROP TABLE IF EXISTS partt1, _partt1_new`)
+	table := `CREATE TABLE partt1 (
+		id INT NOT NULL PRIMARY KEY auto_increment,
+		name varchar(255) NOT NULL
+	)`
+	testutils.RunSQL(t, table)
+	cfg, err := mysql.ParseDSN(testutils.DSN())
+	assert.NoError(t, err)
+	m, err := NewRunner(&Migration{
+		Host:     cfg.Addr,
+		Username: cfg.User,
+		Password: cfg.Passwd,
+		Database: cfg.DBName,
+		Threads:  16,
+		Table:    "partt1",
+		Alter:    "PARTITION BY KEY() PARTITIONS 8",
+	})
+	assert.NoError(t, err)
+	assert.NoError(t, m.Run(context.Background()))
+	assert.NoError(t, m.Close())
+}
+
 func TestVarbinary(t *testing.T) {
 	testutils.RunSQL(t, `DROP TABLE IF EXISTS varbinaryt1, _varbinaryt1_new`)
 	table := `CREATE TABLE varbinaryt1 (