sql: apply ALTER reverse changes from down to top (#811)

go.sum

@@ -14,6 +14,7 @@ github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e h1:fY5BOSpyZCqRo5O
 github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI=
 github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1 h1:q763qf9huN11kDQavWsoZXJNW3xEE4JJyHa5Q25/sd8=
 github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU=
+github.com/cpuguy83/go-md2man/v2 v2.0.1 h1:r/myEWzV9lfsM1tFLgDyu0atFtJ1fXn261LKYj/3DxU=
 github.com/cpuguy83/go-md2man/v2 v2.0.1/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
@@ -57,6 +58,7 @@ github.com/mitchellh/go-wordwrap v0.0.0-20150314170334-ad45545899c7 h1:DpOJ2HYzC
 github.com/mitchellh/go-wordwrap v0.0.0-20150314170334-ad45545899c7/go.mod h1:ZXFpozHsX6DPmq2I0TCekCxypsnAUbP2oI0UX1GXzOo=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
+github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk=
 github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
 github.com/sergi/go-diff v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ=
 github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo=
@@ -107,6 +109,7 @@ google.golang.org/appengine v1.6.5/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCID
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY=
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY=
 gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b h1:h8qDotaEPuJATrMmW04NCwg7v22aHH28wwpauUhK9Oo=

sql/internal/sqlx/sqlx.go

@@ -430,3 +430,10 @@ func ExprLastIndex(expr string) int {
 	}
 	return -1
 }
+
+// ReverseChanges reverses the order of the changes.
+func ReverseChanges(c []schema.Change) {
+	for i, n := 0, len(c); i < n/2; i++ {
+		c[i], c[n-i-1] = c[n-i-1], c[i]
+	}
+}

sql/internal/sqlx/sqlx_test.go

@@ -122,3 +122,61 @@ func TestExprLastIndex(t *testing.T) {
 		})
 	}
 }
+
+func TestReverseChanges(t *testing.T) {
+	tests := []struct {
+		input  []schema.Change
+		expect []schema.Change
+	}{
+		{
+			input: []schema.Change{
+				(*schema.AddColumn)(nil),
+			},
+			expect: []schema.Change{
+				(*schema.AddColumn)(nil),
+			},
+		},
+		{
+			input: []schema.Change{
+				(*schema.AddColumn)(nil),
+				(*schema.DropColumn)(nil),
+			},
+			expect: []schema.Change{
+				(*schema.DropColumn)(nil),
+				(*schema.AddColumn)(nil),
+			},
+		},
+		{
+			input: []schema.Change{
+				(*schema.AddColumn)(nil),
+				(*schema.ModifyColumn)(nil),
+				(*schema.DropColumn)(nil),
+			},
+			expect: []schema.Change{
+				(*schema.DropColumn)(nil),
+				(*schema.ModifyColumn)(nil),
+				(*schema.AddColumn)(nil),
+			},
+		},
+		{
+			input: []schema.Change{
+				(*schema.AddColumn)(nil),
+				(*schema.ModifyColumn)(nil),
+				(*schema.DropColumn)(nil),
+				(*schema.ModifyColumn)(nil),
+			},
+			expect: []schema.Change{
+				(*schema.ModifyColumn)(nil),
+				(*schema.DropColumn)(nil),
+				(*schema.ModifyColumn)(nil),
+				(*schema.AddColumn)(nil),
+			},
+		},
+	}
+	for i, tt := range tests {
+		t.Run(strconv.Itoa(i), func(t *testing.T) {
+			ReverseChanges(tt.input)
+			require.Equal(t, tt.expect, tt.input)
+		})
+	}
+}

sql/mysql/migrate.go

@@ -444,6 +444,9 @@ func (s *state) alterTable(t *schema.Table, changes []schema.Change) error {
 		Comment: fmt.Sprintf("modify %q table", t.Name),
 	}
 	if reversible {
+		// Changes should be reverted in
+		// a reversed order they were created.
+		sqlx.ReverseChanges(reverse)
 		if change.Reverse, err = build(reverse); err != nil {
 			return fmt.Errorf("reversd alter table %q: %v", t.Name, err)
 		}

sql/mysql/migrate_test.go

@@ -485,7 +485,42 @@ func TestPlanChanges(t *testing.T) {
 				Changes: []*migrate.Change{
 					{
 						Cmd:     "ALTER TABLE `users` ADD COLUMN `name` varchar(255) NOT NULL, ADD INDEX `id_key` USING HASH (`id`) COMMENT \"comment\", ADD CONSTRAINT `id_nonzero` CHECK (id > 0) ENFORCED, AUTO_INCREMENT 1000",
-						Reverse: "ALTER TABLE `users` DROP COLUMN `name`, DROP INDEX `id_key`, DROP CONSTRAINT `id_nonzero`, AUTO_INCREMENT 1",
+						Reverse: "ALTER TABLE `users` AUTO_INCREMENT 1, DROP CONSTRAINT `id_nonzero`, DROP INDEX `id_key`, DROP COLUMN `name`",
+					},
+				},
+			},
+		},
+		{
+			changes: []schema.Change{
+				func() schema.Change {
+					users := schema.NewTable("users").
+						AddColumns(schema.NewIntColumn("id", "int"))
+					posts := schema.NewTable("posts").
+						AddColumns(
+							schema.NewIntColumn("id", "int"),
+							schema.NewIntColumn("author_id", "int"),
+						)
+					posts.AddForeignKeys(
+						schema.NewForeignKey("author").
+							AddColumns(posts.Columns[1]).
+							SetRefTable(users).
+							AddRefColumns(users.Columns[0]),
+					)
+					return &schema.ModifyTable{
+						T: posts,
+						Changes: []schema.Change{
+							&schema.AddColumn{C: posts.Columns[1]},
+							&schema.AddForeignKey{F: posts.ForeignKeys[0]},
+						},
+					}
+				}(),
+			},
+			wantPlan: &migrate.Plan{
+				Reversible: true,
+				Changes: []*migrate.Change{
+					{
+						Cmd:     "ALTER TABLE `posts` ADD COLUMN `author_id` int NOT NULL, ADD CONSTRAINT `author` FOREIGN KEY (`author_id`) REFERENCES `users` (`id`)",
+						Reverse: "ALTER TABLE `posts` DROP FOREIGN KEY `author`, DROP COLUMN `author_id`",
 					},
 				},
 			},
@@ -513,7 +548,7 @@ func TestPlanChanges(t *testing.T) {
 				Changes: []*migrate.Change{
 					{
 						Cmd:     "ALTER TABLE `users` ADD COLUMN `c2` int AS (c1*2) NOT NULL, ADD COLUMN `c3` int AS (c1*c2) STORED NOT NULL",
-						Reverse: "ALTER TABLE `users` DROP COLUMN `c2`, DROP COLUMN `c3`",
+						Reverse: "ALTER TABLE `users` DROP COLUMN `c3`, DROP COLUMN `c2`",
 					},
 				},
 			},
@@ -604,7 +639,7 @@ func TestPlanChanges(t *testing.T) {
 				Changes: []*migrate.Change{
 					{
 						Cmd:     "ALTER TABLE `users` ALTER CHECK `check1` ENFORCED, ALTER CHECK `check2` NOT ENFORCED, DROP CHECK `check3`, ADD CONSTRAINT `check3` CHECK (id >= 0)",
-						Reverse: "ALTER TABLE `users` ALTER CHECK `check1` NOT ENFORCED, ALTER CHECK `check2` ENFORCED, DROP CHECK `check3`, ADD CONSTRAINT `check3` CHECK (id > 0)",
+						Reverse: "ALTER TABLE `users` DROP CHECK `check3`, ADD CONSTRAINT `check3` CHECK (id > 0), ALTER CHECK `check2` ENFORCED, ALTER CHECK `check1` NOT ENFORCED",
 					},
 				},
 			},

sql/postgres/migrate.go

@@ -396,6 +396,9 @@ func (s *state) alterTable(t *schema.Table, changes []schema.Change) error {
 		Comment: fmt.Sprintf("modify %q table", t.Name),
 	}
 	if reversible {
+		// Changes should be reverted in
+		// a reversed order they were created.
+		sqlx.ReverseChanges(reverse)
 		if change.Reverse, err = build(reverse); err != nil {
 			return fmt.Errorf("reversd alter table %q: %v", t.Name, err)
 		}

sql/postgres/migrate_test.go

@@ -354,7 +354,7 @@ func TestPlanChanges(t *testing.T) {
 				Changes: []*migrate.Change{
 					{
 						Cmd:     `ALTER TABLE "users" ADD COLUMN "name" character varying(255) NOT NULL DEFAULT 'logged_in', ADD COLUMN "last" character varying(255) NOT NULL DEFAULT 'logged_in', ADD CONSTRAINT "name_not_empty" CHECK ("name" <> ''), DROP CONSTRAINT "id_nonzero", DROP CONSTRAINT "id_iseven", ADD CONSTRAINT "id_iseven" CHECK (("id") % 2 = 0)`,
-						Reverse: `ALTER TABLE "users" DROP COLUMN "name", DROP COLUMN "last", DROP CONSTRAINT "name_not_empty", ADD CONSTRAINT "id_nonzero" CHECK ("id" <> 0), DROP CONSTRAINT "id_iseven", ADD CONSTRAINT "id_iseven" CHECK ("id" % 2 = 0)`,
+						Reverse: `ALTER TABLE "users" DROP CONSTRAINT "id_iseven", ADD CONSTRAINT "id_iseven" CHECK ("id" % 2 = 0), ADD CONSTRAINT "id_nonzero" CHECK ("id" <> 0), DROP CONSTRAINT "name_not_empty", DROP COLUMN "last", DROP COLUMN "name"`,
 					},
 					{
 						Cmd:     `CREATE INDEX "id_key" ON "users" ("id" DESC) WHERE success`,
@@ -409,7 +409,7 @@ func TestPlanChanges(t *testing.T) {
 				Changes: []*migrate.Change{
 					{
 						Cmd:     `ALTER TABLE "users" DROP COLUMN "name", ALTER COLUMN "id" SET GENERATED BY DEFAULT SET START WITH 1024 SET INCREMENT BY 1 RESTART`,
-						Reverse: `ALTER TABLE "users" ADD COLUMN "name" character varying NOT NULL, ALTER COLUMN "id" SET GENERATED BY DEFAULT SET START WITH 1 SET INCREMENT BY 1 RESTART`,
+						Reverse: `ALTER TABLE "users" ALTER COLUMN "id" SET GENERATED BY DEFAULT SET START WITH 1 SET INCREMENT BY 1 RESTART, ADD COLUMN "name" character varying NOT NULL`,
 					},
 				},
 			},
@@ -680,8 +680,10 @@ func TestPlanChanges(t *testing.T) {
 				Reversible:    true,
 				Transactional: true,
 				Changes: []*migrate.Change{
-					{Cmd: `ALTER TABLE "users" ALTER COLUMN "one" DROP DEFAULT, ALTER COLUMN "two" DROP DEFAULT, ALTER COLUMN "id" SET GENERATED BY DEFAULT SET START WITH 1024 SET INCREMENT BY 1 RESTART`,
-						Reverse: `ALTER TABLE "users" ALTER COLUMN "one" SET DEFAULT 'one', ALTER COLUMN "two" SET DEFAULT 'two', ALTER COLUMN "id" SET GENERATED BY DEFAULT SET START WITH 1 SET INCREMENT BY 1 RESTART`},
+					{
+						Cmd:     `ALTER TABLE "users" ALTER COLUMN "one" DROP DEFAULT, ALTER COLUMN "two" DROP DEFAULT, ALTER COLUMN "id" SET GENERATED BY DEFAULT SET START WITH 1024 SET INCREMENT BY 1 RESTART`,
+						Reverse: `ALTER TABLE "users" ALTER COLUMN "id" SET GENERATED BY DEFAULT SET START WITH 1 SET INCREMENT BY 1 RESTART, ALTER COLUMN "two" SET DEFAULT 'two', ALTER COLUMN "one" SET DEFAULT 'one'`,
+					},
 				},
 			},
 		},