Fix column renames for indexes (#147)

Author: bplunkett-stripe

internal/queries/queries.sql

@@ -95,6 +95,13 @@ SELECT
     i.indisunique AS index_is_unique,
     COALESCE(parent_c.relname, '')::TEXT AS parent_index_name,
     COALESCE(parent_namespace.nspname, '')::TEXT AS parent_index_schema_name,
+    (
+        SELECT ARRAY_AGG(att.attname ORDER BY indkey_ord.ord)
+        FROM UNNEST(i.indkey) WITH ORDINALITY AS indkey_ord (attnum, ord)
+        INNER JOIN
+            pg_catalog.pg_attribute AS att
+            ON att.attrelid = table_c.oid AND att.attnum = indkey_ord.attnum
+    )::TEXT [] AS column_names,
     COALESCE(con.conislocal, false) AS constraint_is_local
 FROM pg_catalog.pg_class AS c
 INNER JOIN pg_catalog.pg_index AS i ON (i.indexrelid = c.oid)
@@ -119,14 +126,6 @@ WHERE
     AND table_namespace.nspname !~ '^pg_temp'
     AND (c.relkind = 'i' OR c.relkind = 'I');
 
--- name: GetColumnsForIndex :many
-SELECT a.attname::TEXT AS column_name
-FROM pg_catalog.pg_attribute AS a
-WHERE
-    a.attrelid = $1
-    AND a.attnum > 0
-ORDER BY a.attnum;
-
 -- name: GetCheckConstraints :many
 SELECT
     pg_constraint.oid,

internal/queries/queries.sql.go

@@ -931,31 +931,16 @@ func (s *schemaFetcher) buildCheckConstraint(ctx context.Context, cc queries.Get
 	}, nil
 }
 
-// fetchIndexes fetches the indexes We fetch all indexes at once to minimize number of queries, since each index needs
-// to fetch columns
+// fetchIndexes fetches the indexes. We fetch all the indexes at once to minimize the number of queries.
 func (s *schemaFetcher) fetchIndexes(ctx context.Context) ([]Index, error) {
 	rawIndexes, err := s.q.GetIndexes(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("GetIndexes: %w", err)
 	}
 
-	goroutineRunner := s.goroutineRunnerFactory()
-	var idxFutures []concurrent.Future[Index]
-	for _, _rawIndex := range rawIndexes {
-		rawIndex := _rawIndex // Capture loop variable for go routine
-		f, err := concurrent.SubmitFuture(ctx, goroutineRunner, func() (Index, error) {
-			return s.buildIndex(ctx, rawIndex)
-		})
-		if err != nil {
-			return nil, fmt.Errorf("starting index future: %w", err)
-		}
-
-		idxFutures = append(idxFutures, f)
-	}
-
-	idxs, err := concurrent.GetAll(ctx, idxFutures...)
-	if err != nil {
-		return nil, fmt.Errorf("getting indexes: %w", err)
+	var idxs []Index
+	for _, idx := range rawIndexes {
+		idxs = append(idxs, s.buildIndex(idx))
 	}
 
 	idxs = filterSliceByName(
@@ -969,12 +954,7 @@ func (s *schemaFetcher) fetchIndexes(ctx context.Context) ([]Index, error) {
 	return idxs, nil
 }
 
-func (s *schemaFetcher) buildIndex(ctx context.Context, rawIndex queries.GetIndexesRow) (Index, error) {
-	rawColumns, err := s.q.GetColumnsForIndex(ctx, rawIndex.Oid)
-	if err != nil {
-		return Index{}, fmt.Errorf("GetColumnsForIndex(%s): %w", rawIndex.Oid, err)
-	}
-
+func (s *schemaFetcher) buildIndex(rawIndex queries.GetIndexesRow) Index {
 	var indexConstraint *IndexConstraint
 	if rawIndex.ConstraintName != "" {
 		indexConstraint = &IndexConstraint{
@@ -999,15 +979,15 @@ func (s *schemaFetcher) buildIndex(ctx context.Context, rawIndex queries.GetInde
 			EscapedName: EscapeIdentifier(rawIndex.TableName),
 		},
 		Name:            rawIndex.IndexName,
-		Columns:         rawColumns,
+		Columns:         rawIndex.ColumnNames,
 		GetIndexDefStmt: GetIndexDefStatement(rawIndex.DefStmt),
 		IsInvalid:       !rawIndex.IndexIsValid,
 		IsUnique:        rawIndex.IndexIsUnique,
 
 		Constraint: indexConstraint,
 
 		ParentIdx: parentIdx,
-	}, nil
+	}
 }
 
 func (s *schemaFetcher) fetchForeignKeyCons(ctx context.Context) ([]ForeignKeyConstraint, error) {

internal/schema/schema.go

@@ -892,6 +892,38 @@ var (
 				},
 			},
 		},
+		{
+			name: "Column rename",
+			ddl: []string{`
+			CREATE TABLE foo (
+				value TEXT
+			);
+			CREATE INDEX foo_value_idx ON foo (value);
+			ALTER TABLE foo RENAME COLUMN value TO renamed_value;
+		`},
+			expectedSchema: Schema{
+				NamedSchemas: []NamedSchema{
+					{Name: "public"},
+				},
+				Tables: []Table{
+					{
+						SchemaQualifiedName: SchemaQualifiedName{SchemaName: "public", EscapedName: "\"foo\""},
+						Columns: []Column{
+							{Name: "renamed_value", Type: "text", IsNullable: true, Size: -1, Collation: defaultCollation},
+						},
+						CheckConstraints: nil,
+						ReplicaIdentity:  ReplicaIdentityDefault,
+					},
+				},
+				Indexes: []Index{
+					{
+						OwningTable: SchemaQualifiedName{SchemaName: "public", EscapedName: "\"foo\""},
+						Name:        "foo_value_idx", Columns: []string{"renamed_value"},
+						GetIndexDefStmt: "CREATE INDEX foo_value_idx ON public.foo USING btree (renamed_value)",
+					},
+				},
+			},
+		},
 		{
 			name: "Filtering - filtering out the base table",
 			opts: []GetSchemaOpt{WithIncludeSchemas("public")},

internal/schema/schema_test.go

@@ -1718,6 +1718,7 @@ func (isg *indexSQLVertexGenerator) addDepsOnTableAddAlterIfNecessary(index sche
 		}
 	}
 
+	// Otherwise, we can drop the index whenever we want.
 	return nil
 }