sql/postgres: move resource drop to the end of the planning

ariga · Jul 9, 2023 · 0ab2c8c · 0ab2c8c
1 parent 2d2a836
commit 0ab2c8c
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 23 deletions.
diff --git a/internal/integration/postgres_test.go b/internal/integration/postgres_test.go
@@ -996,10 +996,10 @@ CREATE TABLE "other"."posts" ("id" integer NOT NULL);`, diff("test1?sslmode=disa
 		// Diffing schema should both tables and comments (from 'public' to 'other').
 		require.Equal(t, `-- Set comment to schema: ""
 COMMENT ON SCHEMA IS 'standard public schema';
--- Drop "posts" table
-DROP TABLE "posts";
 -- Create "users" table
-CREATE TABLE "users" ("id" integer NOT NULL);`, diff("test2?sslmode=disable&search_path=other", "test2?sslmode=disable&search_path=public"))
+CREATE TABLE "users" ("id" integer NOT NULL);
+-- Drop "posts" table
+DROP TABLE "posts";`, diff("test2?sslmode=disable&search_path=other", "test2?sslmode=disable&search_path=public"))
 		// diff between schema and database
 		out, err = exec.Command(
 			bin, "schema", "diff",

diff --git a/sql/postgres/migrate.go b/sql/postgres/migrate.go
@@ -78,31 +78,36 @@ func (s *state) plan(ctx context.Context, changes []schema.Change) error {
 	if planned, err = sqlx.DetachCycles(planned); err != nil {
 		return err
 	}
-	deferred := make([]schema.Change, 0, len(planned))
+	var (
+		views []schema.Change
+		dropT []*schema.DropTable
+		dropO []*schema.DropObject
+	)
 	for _, c := range planned {
 		switch c := c.(type) {
 		case *schema.AddTable:
 			err = s.addTable(ctx, c)
-		case *schema.DropTable:
-			err = s.dropTable(ctx, c)
 		case *schema.ModifyTable:
 			err = s.modifyTable(ctx, c)
 		case *schema.RenameTable:
 			s.renameTable(c)
-		case *schema.DropObject, *schema.AddView, *schema.DropView,
-			*schema.ModifyView, *schema.RenameView:
-			deferred = append(deferred, c)
+		case *schema.AddView, *schema.DropView, *schema.ModifyView, *schema.RenameView:
+			views = append(views, c)
+		case *schema.DropTable:
+			dropT = append(dropT, c)
+		case *schema.DropObject:
+			dropO = append(dropO, c)
 		default:
 			err = fmt.Errorf("unsupported change %T", c)
 		}
 		if err != nil {
 			return err
 		}
 	}
-	if deferred, err = sqlx.PlanViewChanges(deferred); err != nil {
+	if views, err = sqlx.PlanViewChanges(views); err != nil {
 		return err
 	}
-	for _, c := range deferred {
+	for _, c := range views {
 		switch c := c.(type) {
 		case *schema.AddView:
 			err = s.addView(ctx, c)
@@ -112,20 +117,26 @@ func (s *state) plan(ctx context.Context, changes []schema.Change) error {
 			err = s.modifyView(ctx, c)
 		case *schema.RenameView:
 			s.renameView(ctx, c)
-		case *schema.DropObject:
-			e, ok := c.O.(*schema.EnumType)
-			if !ok {
-				return fmt.Errorf("unsupported object %T", c.O)
-			}
-			create, drop := s.createDropEnum(e)
-			s.append(&migrate.Change{
-				Source:  c,
-				Cmd:     drop,
-				Reverse: create,
-				Comment: fmt.Sprintf("drop enum type %q", e.T),
-			})
 		}
 	}
+	for _, c := range dropT {
+		if err := s.dropTable(ctx, c); err != nil {
+			return err
+		}
+	}
+	for _, c := range dropO {
+		e, ok := c.O.(*schema.EnumType)
+		if !ok {
+			return fmt.Errorf("unsupported object %T", c.O)
+		}
+		create, drop := s.createDropEnum(e)
+		s.append(&migrate.Change{
+			Source:  c,
+			Cmd:     drop,
+			Reverse: create,
+			Comment: fmt.Sprintf("drop enum type %q", e.T),
+		})
+	}
 	return nil
 }
 

diff --git a/sql/postgres/migrate_test.go b/sql/postgres/migrate_test.go
@@ -366,6 +366,35 @@ func TestPlanChanges(t *testing.T) {
 				},
 			},
 		},
+		{
+			changes: []schema.Change{
+				&schema.DropObject{O: &schema.EnumType{T: "status", Values: []string{"on", "off"}, Schema: schema.New("public")}},
+				&schema.DropTable{T: schema.NewTable("logs").
+					AddColumns(
+						schema.NewColumn("status").SetType(&schema.EnumType{T: "status", Values: []string{"on", "off"}, Schema: schema.New("public")}),
+					),
+				},
+				&schema.DropObject{O: &schema.EnumType{T: "state", Values: []string{"on", "off"}, Schema: schema.New("public")}},
+			},
+			wantPlan: &migrate.Plan{
+				Reversible:    true,
+				Transactional: true,
+				Changes: []*migrate.Change{
+					{
+						Cmd:     `DROP TABLE "logs"`,
+						Reverse: `CREATE TABLE "logs" ("status" "public"."status" NOT NULL)`,
+					},
+					{
+						Cmd:     `DROP TYPE "public"."status"`,
+						Reverse: `CREATE TYPE "public"."status" AS ENUM ('on', 'off')`,
+					},
+					{
+						Cmd:     `DROP TYPE "public"."state"`,
+						Reverse: `CREATE TYPE "public"."state" AS ENUM ('on', 'off')`,
+					},
+				},
+			},
+		},
 		{
 			changes: []schema.Change{
 				&schema.DropTable{