Repository: ariga/atlas
Pull request #1557 (2023-03-26 07:54:15)

sql/postgres: change the Concurrently attribute to a schema.Clause
The `Concurrently` option only affects how the index is created and doesn't provide any information about the index itself. 

cc @ivanvanderbyl 


Patch:
diff --git a/sql/postgres/inspect.go b/sql/postgres/inspect.go
index 3c8826a3..89ea7c41 100644
--- a/sql/postgres/inspect.go
+++ b/sql/postgres/inspect.go
@@ -830,7 +830,7 @@ type (
 	// build or drop the index concurrently without blocking the current table.
 	// https://www.postgresql.org/docs/current/sql-createindex.html#SQL-CREATEINDEX-CONCURRENTLY
 	Concurrently struct {
-		schema.Attr
+		schema.Clause
 	}
 
 	// NoInherit attribute defines the NO INHERIT flag for CHECK constraint.
diff --git a/sql/postgres/migrate.go b/sql/postgres/migrate.go
index d695bb44..3d32ab97 100644
--- a/sql/postgres/migrate.go
+++ b/sql/postgres/migrate.go
@@ -192,8 +192,11 @@ func (s *state) addTable(ctx context.Context, add *schema.AddTable) error {
 		Comment: fmt.Sprintf("create %q table", add.T.Name),
 		Reverse: s.Build("DROP TABLE").Table(add.T).String(),
 	})
-	if err := s.addIndexes(add.T, add.T.Indexes...); err != nil {
-		return err
+	for _, idx := range add.T.Indexes {
+		// Indexes do not need to be created concurrently on new tables.
+		if err := s.addIndexes(add.T, &schema.AddIndex{I: idx}); err != nil {
+			return err
+		}
 	}
 	s.addComments(add.T)
 	return nil
@@ -252,9 +255,10 @@ func (s *state) dropTable(ctx context.Context, drop *schema.DropTable) error {
 // modifyTable builds the statements that bring the table into its modified state.
 func (s *state) modifyTable(ctx context.Context, modify *schema.ModifyTable) error {
 	var (
-		alter       []schema.Change
-		addI, dropI []*schema.Index
-		changes     []*migrate.Change
+		alter   []schema.Change
+		addI    []*schema.AddIndex
+		dropI   []*schema.DropIndex
+		changes []*migrate.Change
 	)
 	for _, change := range skipAutoChanges(modify.Changes) {
 		switch change := change.(type) {
@@ -270,14 +274,14 @@ func (s *state) modifyTable(ctx context.Context, modify *schema.ModifyTable) err
 			if c := (schema.Comment{}); sqlx.Has(change.I.Attrs, &c) {
 				changes = append(changes, s.indexComment(modify.T, change.I, c.Text, ""))
 			}
-			addI = append(addI, change.I)
+			addI = append(addI, change)
 		case *schema.DropIndex:
 			// Unlike DROP INDEX statements that are executed separately,
 			// DROP CONSTRAINT are added to the ALTER TABLE statement below.
 			if isUniqueConstraint(change.I) {
 				alter = append(alter, change)
 			} else {
-				dropI = append(dropI, change.I)
+				dropI = append(dropI, change)
 			}
 		case *schema.ModifyIndex:
 			k := change.Change
@@ -293,8 +297,8 @@ func (s *state) modifyTable(ctx context.Context, modify *schema.ModifyTable) err
 				}
 			}
 			// Index modification requires rebuilding the index.
-			addI = append(addI, change.To)
-			dropI = append(dropI, change.From)
+			addI = append(addI, &schema.AddIndex{I: change.To})
+			dropI = append(dropI, &schema.DropIndex{I: change.From})
 		case *schema.RenameIndex:
 			changes = append(changes, &migrate.Change{
 				Source:  change,
@@ -729,15 +733,19 @@ func (s *state) indexComment(t *schema.Table, idx *schema.Index, to, from string
 	}
 }
 
-func (s *state) dropIndexes(t *schema.Table, indexes ...*schema.Index) error {
+func (s *state) dropIndexes(t *schema.Table, drops ...*schema.DropIndex) error {
+	adds := make([]*schema.AddIndex, len(drops))
+	for i, d := range drops {
+		adds[i] = &schema.AddIndex{I: d.I, Extra: d.Extra}
+	}
 	rs := &state{conn: s.conn, PlanOptions: s.PlanOptions}
-	if err := rs.addIndexes(t, indexes...); err != nil {
+	if err := rs.addIndexes(t, adds...); err != nil {
 		return err
 	}
-	for i, idx := range indexes {
+	for i, add := range adds {
 		s.append(&migrate.Change{
 			Cmd:     rs.Changes[i].Reverse.(string),
-			Comment: fmt.Sprintf("drop index %q from table: %q", idx.Name, t.Name),
+			Comment: fmt.Sprintf("drop index %q from table: %q", add.I.Name, t.Name),
 			Reverse: rs.Changes[i].Cmd,
 		})
 	}
@@ -856,14 +864,14 @@ func (s *state) mayDropEnum(cmd *changeGroup, ns *schema.Schema, e *schema.EnumT
 	return nil
 }
 
-func (s *state) addIndexes(t *schema.Table, indexes ...*schema.Index) error {
-	for _, idx := range indexes {
-		b := s.Build("CREATE")
+func (s *state) addIndexes(t *schema.Table, adds ...*schema.AddIndex) error {
+	for _, add := range adds {
+		b, idx := s.Build("CREATE"), add.I
 		if idx.Unique {
 			b.P("UNIQUE")
 		}
 		b.P("INDEX")
-		if c := (Concurrently{}); sqlx.Has(idx.Attrs, &c) {
+		if sqlx.Has(add.Extra, &Concurrently{}) {
 			b.P("CONCURRENTLY")
 		}
 		if idx.Name != "" {
@@ -878,7 +886,7 @@ func (s *state) addIndexes(t *schema.Table, indexes ...*schema.Index) error {
 			Comment: fmt.Sprintf("create index %q to table: %q", idx.Name, t.Name),
 			Reverse: func() string {
 				b := s.Build("DROP INDEX")
-				if c := (Concurrently{}); sqlx.Has(idx.Attrs, &c) {
+				if sqlx.Has(add.Extra, &Concurrently{}) {
 					b.P("CONCURRENTLY")
 				}
 				// Unlike MySQL, the DROP command is not attached to ALTER TABLE.
@@ -1050,7 +1058,7 @@ func (s *state) index(b *sqlx.Builder, idx *schema.Index) error {
 	}
 	for _, attr := range idx.Attrs {
 		switch attr.(type) {
-		case *schema.Comment, *IndexType, *IndexInclude, *Concurrently, *Constraint, *IndexPredicate, *IndexStorageParams:
+		case *schema.Comment, *IndexType, *IndexInclude, *Constraint, *IndexPredicate, *IndexStorageParams:
 		default:
 			return fmt.Errorf("postgres: unexpected index attribute: %T", attr)
 		}
diff --git a/sql/schema/migrate.go b/sql/schema/migrate.go
index ef172c0c..45958a0c 100644
--- a/sql/schema/migrate.go
+++ b/sql/schema/migrate.go
@@ -109,12 +109,14 @@ type (
 
 	// AddIndex describes an index creation change.
 	AddIndex struct {
-		I *Index
+		I     *Index
+		Extra []Clause // Extra clauses and options.
 	}
 
 	// DropIndex describes an index removal change.
 	DropIndex struct {
-		I *Index
+		I     *Index
+		Extra []Clause // Extra clauses and options.
 	}
 
 	// ModifyIndex describes an index modification.


Test patch:
diff --git a/sql/postgres/migrate_test.go b/sql/postgres/migrate_test.go
index 8729c1f8..96c67ac0 100644
--- a/sql/postgres/migrate_test.go
+++ b/sql/postgres/migrate_test.go
@@ -534,14 +534,25 @@ func TestPlanChanges(t *testing.T) {
 							},
 							&schema.AddIndex{
 								I: &schema.Index{
-									Name: "concurrently",
+									Name: "add_con",
 									Parts: []*schema.IndexPart{
 										{C: users.Columns[0]},
 									},
-									Attrs: []schema.Attr{
-										&Concurrently{},
+								},
+								Extra: []schema.Clause{
+									&Concurrently{},
+								},
+							},
+							&schema.DropIndex{
+								I: &schema.Index{
+									Name: "drop_con",
+									Parts: []*schema.IndexPart{
+										{C: users.Columns[0]},
 									},
 								},
+								Extra: []schema.Clause{
+									&Concurrently{},
+								},
 							},
 							&schema.AddIndex{
 								I: &schema.Index{
@@ -564,6 +575,10 @@ func TestPlanChanges(t *testing.T) {
 				Reversible:    true,
 				Transactional: true,
 				Changes: []*migrate.Change{
+					{
+						Cmd:     `DROP INDEX CONCURRENTLY "drop_con"`,
+						Reverse: `CREATE INDEX CONCURRENTLY "drop_con" ON "users" ("id")`,
+					},
 					{
 						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 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"`,
@@ -581,8 +596,8 @@ func TestPlanChanges(t *testing.T) {
 						Reverse: `DROP INDEX "include_key"`,
 					},
 					{
-						Cmd:     `CREATE INDEX CONCURRENTLY "concurrently" ON "users" ("id")`,
-						Reverse: `DROP INDEX CONCURRENTLY "concurrently"`,
+						Cmd:     `CREATE INDEX CONCURRENTLY "add_con" ON "users" ("id")`,
+						Reverse: `DROP INDEX CONCURRENTLY "add_con"`,
 					},
 					{
 						Cmd:     `CREATE INDEX "operator_class" ON "users" USING BRIN ("id" int8_bloom_ops, "id", "id" int8_minmax_multi_ops(values_per_range=8))`,