Repository: ariga/atlas
Pull request #734 (2022-04-24 18:00:53)

sql: minor RENAME COLUMN fixes in ent integration


Patch:
diff --git a/sql/mysql/driver.go b/sql/mysql/driver.go
index 80175271..41d44868 100644
--- a/sql/mysql/driver.go
+++ b/sql/mysql/driver.go
@@ -170,6 +170,16 @@ func (d *conn) supportsGeneratedColumns() bool {
 	return d.gteV(v)
 }
 
+// supportsRenameColumn reports if the connected database
+// supports the "RENAME COLUMN" clause.
+func (d *conn) supportsRenameColumn() bool {
+	v := "8"
+	if d.mariadb() {
+		v = "10.5.2"
+	}
+	return d.gteV(v)
+}
+
 // mariadb reports if the Driver is connected to a MariaDB database.
 func (d *conn) mariadb() bool {
 	return strings.Index(d.version, "MariaDB") > 0
diff --git a/sql/mysql/migrate.go b/sql/mysql/migrate.go
index c711f22c..ec3496c1 100644
--- a/sql/mysql/migrate.go
+++ b/sql/mysql/migrate.go
@@ -349,7 +349,14 @@ func (s *state) alterTable(t *schema.Table, changes []schema.Change) error {
 					Change: change.Change,
 				})
 			case *schema.RenameColumn:
-				b.P("RENAME COLUMN").Ident(change.From.Name).P("TO").Ident(change.To.Name)
+				if s.supportsRenameColumn() {
+					b.P("RENAME COLUMN").Ident(change.From.Name).P("TO").Ident(change.To.Name)
+				} else {
+					b.P("CHANGE COLUMN").Ident(change.From.Name)
+					if err := s.column(b, t, change.To); err != nil {
+						return err
+					}
+				}
 				reverse = append(reverse, &schema.RenameColumn{From: change.To, To: change.From})
 			case *schema.DropColumn:
 				b.P("DROP COLUMN").Ident(change.C.Name)
diff --git a/sql/postgres/migrate.go b/sql/postgres/migrate.go
index 704cd5d3..9edaac72 100644
--- a/sql/postgres/migrate.go
+++ b/sql/postgres/migrate.go
@@ -184,9 +184,9 @@ func (s *state) dropTable(drop *schema.DropTable) {
 // modifyTable builds the statements that bring the table into its modified state.
 func (s *state) modifyTable(ctx context.Context, modify *schema.ModifyTable) error {
 	var (
-		changes     []schema.Change
+		alter       []schema.Change
 		addI, dropI []*schema.Index
-		comments    []*migrate.Change
+		changes     []*migrate.Change
 	)
 	for _, change := range skipAutoChanges(modify.Changes) {
 		switch change := change.(type) {
@@ -195,12 +195,12 @@ func (s *state) modifyTable(ctx context.Context, modify *schema.ModifyTable) err
 			if err != nil {
 				return err
 			}
-			comments = append(comments, s.tableComment(modify.T, to, from))
+			changes = append(changes, s.tableComment(modify.T, to, from))
 		case *schema.DropAttr:
 			return fmt.Errorf("unsupported change type: %T", change)
 		case *schema.AddIndex:
 			if c := (schema.Comment{}); sqlx.Has(change.I.Attrs, &c) {
-				comments = append(comments, s.indexComment(modify.T, change.I, c.Text, ""))
+				changes = append(changes, s.indexComment(modify.T, change.I, c.Text, ""))
 			}
 			addI = append(addI, change.I)
 		case *schema.DropIndex:
@@ -212,7 +212,7 @@ func (s *state) modifyTable(ctx context.Context, modify *schema.ModifyTable) err
 				if err != nil {
 					return err
 				}
-				comments = append(comments, s.indexComment(modify.T, change.To, to, from))
+				changes = append(changes, s.indexComment(modify.T, change.To, to, from))
 				// If only the comment of the index was changed.
 				if k &= ^schema.ChangeComment; k.Is(schema.NoChange) {
 					continue
@@ -222,7 +222,7 @@ func (s *state) modifyTable(ctx context.Context, modify *schema.ModifyTable) err
 			addI = append(addI, change.To)
 			dropI = append(dropI, change.From)
 		case *schema.RenameIndex:
-			s.append(&migrate.Change{
+			changes = append(changes, &migrate.Change{
 				Source:  change,
 				Comment: fmt.Sprintf("rename an index from %q to %q", change.From.Name, change.To.Name),
 				Cmd:     Build("ALTER INDEX").Ident(change.From.Name).P("RENAME TO").Ident(change.To.Name).String(),
@@ -231,7 +231,7 @@ func (s *state) modifyTable(ctx context.Context, modify *schema.ModifyTable) err
 		case *schema.ModifyForeignKey:
 			// Foreign-key modification is translated into 2 steps.
 			// Dropping the current foreign key and creating a new one.
-			changes = append(changes, &schema.DropForeignKey{
+			alter = append(alter, &schema.DropForeignKey{
 				F: change.From,
 			}, &schema.AddForeignKey{
 				F: change.To,
@@ -241,9 +241,9 @@ func (s *state) modifyTable(ctx context.Context, modify *schema.ModifyTable) err
 				return err
 			}
 			if c := (schema.Comment{}); sqlx.Has(change.C.Attrs, &c) {
-				comments = append(comments, s.columnComment(modify.T, change.C, c.Text, ""))
+				changes = append(changes, s.columnComment(modify.T, change.C, c.Text, ""))
 			}
-			changes = append(changes, change)
+			alter = append(alter, change)
 		case *schema.ModifyColumn:
 			k := change.Change
 			if change.Change.Is(schema.ChangeComment) {
@@ -251,7 +251,7 @@ func (s *state) modifyTable(ctx context.Context, modify *schema.ModifyTable) err
 				if err != nil {
 					return err
 				}
-				comments = append(comments, s.columnComment(modify.T, change.To, to, from))
+				changes = append(changes, s.columnComment(modify.T, change.To, to, from))
 				// If only the comment of the column was changed.
 				if k &= ^schema.ChangeComment; k.Is(schema.NoChange) {
 					continue
@@ -276,19 +276,29 @@ func (s *state) modifyTable(ctx context.Context, modify *schema.ModifyTable) err
 					return err
 				}
 			}
-			changes = append(changes, change)
+			alter = append(alter, change)
+		case *schema.RenameColumn:
+			// "RENAME COLUMN" cannot be combined with other alterations.
+			b := Build("ALTER TABLE").Table(modify.T).P("RENAME COLUMN")
+			r := b.Clone()
+			changes = append(changes, &migrate.Change{
+				Source:  change,
+				Comment: fmt.Sprintf("rename a column from %q to %q", change.From.Name, change.To.Name),
+				Cmd:     b.Ident(change.From.Name).P("TO").Ident(change.To.Name).String(),
+				Reverse: r.Ident(change.To.Name).P("TO").Ident(change.From.Name).String(),
+			})
 		default:
-			changes = append(changes, change)
+			alter = append(alter, change)
 		}
 	}
 	s.dropIndexes(modify.T, dropI...)
-	if len(changes) > 0 {
-		if err := s.alterTable(modify.T, changes); err != nil {
+	if len(alter) > 0 {
+		if err := s.alterTable(modify.T, alter); err != nil {
 			return err
 		}
 	}
 	s.addIndexes(modify.T, addI...)
-	s.append(comments...)
+	s.append(changes...)
 	return nil
 }
 
@@ -320,9 +330,6 @@ func (s *state) alterTable(t *schema.Table, changes []schema.Change) error {
 					To:     change.From,
 					Change: change.Change & ^schema.ChangeGenerated,
 				})
-			case *schema.RenameColumn:
-				b.P("RENAME COLUMN").Ident(change.From.Name).P("TO").Ident(change.To.Name)
-				reverse = append(reverse, &schema.RenameColumn{From: change.To, To: change.From})
 			case *schema.DropColumn:
 				b.P("DROP COLUMN").Ident(change.C.Name)
 				reverse = append(reverse, &schema.AddColumn{C: change.C})


Test patch:
diff --git a/sql/mysql/migrate_test.go b/sql/mysql/migrate_test.go
index 8577704a..5c4b29b0 100644
--- a/sql/mysql/migrate_test.go
+++ b/sql/mysql/migrate_test.go
@@ -173,12 +173,13 @@ func TestMigrate_DetachCycles(t *testing.T) {
 
 func TestPlanChanges(t *testing.T) {
 	tests := []struct {
-		input    []schema.Change
+		version  string
+		changes  []schema.Change
 		wantPlan *migrate.Plan
 		wantErr  bool
 	}{
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				&schema.AddSchema{S: schema.New("test").SetCharset("utf8mb4"), Extra: []schema.Clause{&schema.IfNotExists{}}},
 				&schema.DropSchema{S: schema.New("test").SetCharset("utf8mb4"), Extra: []schema.Clause{&schema.IfExists{}}},
 			},
@@ -196,7 +197,7 @@ func TestPlanChanges(t *testing.T) {
 			},
 		},
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				func() schema.Change {
 					users := &schema.Table{
 						Name: "users",
@@ -233,7 +234,7 @@ func TestPlanChanges(t *testing.T) {
 			},
 		},
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				func() schema.Change {
 					users := &schema.Table{
 						Name: "users",
@@ -283,7 +284,7 @@ func TestPlanChanges(t *testing.T) {
 			},
 		},
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				&schema.AddSchema{S: &schema.Schema{Name: "test", Attrs: []schema.Attr{&schema.Charset{V: "latin"}}}},
 			},
 			wantPlan: &migrate.Plan{
@@ -293,7 +294,7 @@ func TestPlanChanges(t *testing.T) {
 		},
 		// Default database charset can be omitted.
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				&schema.AddSchema{S: schema.New("test").SetCharset("utf8")},
 			},
 			wantPlan: &migrate.Plan{
@@ -303,7 +304,7 @@ func TestPlanChanges(t *testing.T) {
 		},
 		// Add the default database charset on modify can be omitted.
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				&schema.ModifySchema{
 					S: schema.New("test").SetCharset("utf8"),
 					Changes: []schema.Change{
@@ -317,7 +318,7 @@ func TestPlanChanges(t *testing.T) {
 		},
 		// Add custom charset.
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				&schema.ModifySchema{
 					S: schema.New("test").SetCharset("latin1"),
 					Changes: []schema.Change{
@@ -332,7 +333,7 @@ func TestPlanChanges(t *testing.T) {
 		},
 		// Modify charset.
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				&schema.ModifySchema{
 					S: schema.New("test").SetCharset("utf8"),
 					Changes: []schema.Change{
@@ -346,7 +347,7 @@ func TestPlanChanges(t *testing.T) {
 			},
 		},
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				&schema.DropSchema{S: &schema.Schema{Name: "atlas", Attrs: []schema.Attr{&schema.Charset{V: "latin"}}}},
 			},
 			wantPlan: &migrate.Plan{
@@ -354,7 +355,7 @@ func TestPlanChanges(t *testing.T) {
 			},
 		},
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				func() *schema.AddTable {
 					t := &schema.Table{
 						Name: "posts",
@@ -374,7 +375,7 @@ func TestPlanChanges(t *testing.T) {
 			},
 		},
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				func() *schema.AddTable {
 					t := &schema.Table{
 						Name: "posts",
@@ -410,7 +411,7 @@ func TestPlanChanges(t *testing.T) {
 			},
 		},
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				func() *schema.AddTable {
 					t := &schema.Table{
 						Name: "posts",
@@ -430,7 +431,7 @@ func TestPlanChanges(t *testing.T) {
 			},
 		},
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				&schema.DropTable{T: &schema.Table{Name: "posts"}},
 			},
 			wantPlan: &migrate.Plan{
@@ -438,7 +439,7 @@ func TestPlanChanges(t *testing.T) {
 			},
 		},
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				func() schema.Change {
 					users := &schema.Table{
 						Name: "users",
@@ -490,7 +491,7 @@ func TestPlanChanges(t *testing.T) {
 			},
 		},
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				func() schema.Change {
 					users := schema.NewTable("users").
 						AddColumns(schema.NewIntColumn("c1", "int"))
@@ -518,7 +519,7 @@ func TestPlanChanges(t *testing.T) {
 			},
 		},
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				func() schema.Change {
 					users := &schema.Table{
 						Name: "users",
@@ -551,7 +552,7 @@ func TestPlanChanges(t *testing.T) {
 			},
 		},
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				func() schema.Change {
 					users := &schema.Table{
 						Name: "users",
@@ -609,7 +610,7 @@ func TestPlanChanges(t *testing.T) {
 			},
 		},
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				&schema.AddTable{
 					T: schema.NewTable("users").AddColumns(schema.NewIntColumn("id", "bigint").SetCharset("utf8mb4")),
 				},
@@ -617,7 +618,7 @@ func TestPlanChanges(t *testing.T) {
 			wantErr: true,
 		},
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				&schema.AddTable{
 					T: schema.NewTable("users").AddColumns(schema.NewIntColumn("id", "bigint").SetCollation("utf8mb4_general_ci")),
 				},
@@ -626,7 +627,7 @@ func TestPlanChanges(t *testing.T) {
 		},
 		// Changing a regular column to a VIRTUAL generated column is not allowed.
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				&schema.ModifyTable{
 					T: schema.NewTable("users"),
 					Changes: []schema.Change{
@@ -642,7 +643,7 @@ func TestPlanChanges(t *testing.T) {
 		},
 		// Changing a VIRTUAL generated column to a regular column is not allowed.
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				&schema.ModifyTable{
 					T: schema.NewTable("users"),
 					Changes: []schema.Change{
@@ -658,7 +659,7 @@ func TestPlanChanges(t *testing.T) {
 		},
 		// Changing the storage type of generated column is not allowed.
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				&schema.ModifyTable{
 					T: schema.NewTable("users"),
 					Changes: []schema.Change{
@@ -674,7 +675,7 @@ func TestPlanChanges(t *testing.T) {
 		},
 		// Changing a STORED generated column to a regular column.
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				&schema.ModifyTable{
 					T: schema.NewTable("users"),
 					Changes: []schema.Change{
@@ -698,7 +699,7 @@ func TestPlanChanges(t *testing.T) {
 		},
 		// Changing a regular column to a STORED generated column.
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				&schema.ModifyTable{
 					T: schema.NewTable("users"),
 					Changes: []schema.Change{
@@ -721,7 +722,7 @@ func TestPlanChanges(t *testing.T) {
 			},
 		},
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				&schema.RenameTable{
 					From: schema.NewTable("t1"),
 					To:   schema.NewTable("t2"),
@@ -738,7 +739,7 @@ func TestPlanChanges(t *testing.T) {
 			},
 		},
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				&schema.RenameTable{
 					From: schema.NewTable("t1").SetSchema(schema.New("s1")),
 					To:   schema.NewTable("t2").SetSchema(schema.New("s2")),
@@ -755,7 +756,7 @@ func TestPlanChanges(t *testing.T) {
 			},
 		},
 		{
-			input: []schema.Change{
+			changes: []schema.Change{
 				&schema.ModifyTable{
 					T: schema.NewTable("t1").SetSchema(schema.New("s1")),
 					Changes: []schema.Change{
@@ -777,7 +778,30 @@ func TestPlanChanges(t *testing.T) {
 			},
 		},
 		{
-			input: []schema.Change{
+			version: "5.6",
+			changes: []schema.Change{
+				&schema.ModifyTable{
+					T: schema.NewTable("t1").SetSchema(schema.New("s1")),
+					Changes: []schema.Change{
+						&schema.RenameColumn{
+							From: schema.NewIntColumn("a", "int"),
+							To:   schema.NewIntColumn("b", "int"),
+						},
+					},
+				},
+			},
+			wantPlan: &migrate.Plan{
+				Reversible: true,
+				Changes: []*migrate.Change{
+					{
+						Cmd:     "ALTER TABLE `s1`.`t1` CHANGE COLUMN `a` `b` int NOT NULL",
+						Reverse: "ALTER TABLE `s1`.`t1` CHANGE COLUMN `b` `a` int NOT NULL",
+					},
+				},
+			},
+		},
+		{
+			changes: []schema.Change{
 				&schema.ModifyTable{
 					T: schema.NewTable("t1").SetSchema(schema.New("s1")),
 					Changes: []schema.Change{
@@ -801,9 +825,12 @@ func TestPlanChanges(t *testing.T) {
 	}
 	for i, tt := range tests {
 		t.Run(strconv.Itoa(i), func(t *testing.T) {
-			db, _, err := newMigrate("8.0.16")
+			if tt.version == "" {
+				tt.version = "8.0.16"
+			}
+			db, _, err := newMigrate(tt.version)
 			require.NoError(t, err)
-			plan, err := db.PlanChanges(context.Background(), "wantPlan", tt.input)
+			plan, err := db.PlanChanges(context.Background(), "wantPlan", tt.changes)
 			if tt.wantErr {
 				require.Error(t, err, "expect plan to fail")
 				return
diff --git a/sql/postgres/migrate_test.go b/sql/postgres/migrate_test.go
index ed33b7bf..dfebe0b0 100644
--- a/sql/postgres/migrate_test.go
+++ b/sql/postgres/migrate_test.go
@@ -709,6 +709,36 @@ func TestPlanChanges(t *testing.T) {
 				},
 			},
 		},
+		{
+			changes: []schema.Change{
+				&schema.ModifyTable{
+					T: schema.NewTable("t1").SetSchema(schema.New("s1")),
+					Changes: []schema.Change{
+						&schema.RenameColumn{
+							From: schema.NewColumn("a"),
+							To:   schema.NewColumn("b"),
+						},
+						&schema.AddColumn{
+							C: schema.NewIntColumn("c", "int"),
+						},
+					},
+				},
+			},
+			plan: &migrate.Plan{
+				Reversible:    true,
+				Transactional: true,
+				Changes: []*migrate.Change{
+					{
+						Cmd:     `ALTER TABLE "s1"."t1" ADD COLUMN "c" integer NOT NULL`,
+						Reverse: `ALTER TABLE "s1"."t1" DROP COLUMN "c"`,
+					},
+					{
+						Cmd:     `ALTER TABLE "s1"."t1" RENAME COLUMN "a" TO "b"`,
+						Reverse: `ALTER TABLE "s1"."t1" RENAME COLUMN "b" TO "a"`,
+					},
+				},
+			},
+		},
 		{
 			changes: []schema.Change{
 				&schema.ModifyTable{