Skip to content

Commit

Permalink
Fix column renames for indexes (#147)
Browse files Browse the repository at this point in the history
  • Loading branch information
bplunkett-stripe authored Jul 19, 2024
1 parent 7b45b90 commit 4fc0bff
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 67 deletions.
15 changes: 7 additions & 8 deletions internal/queries/queries.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand Down
41 changes: 9 additions & 32 deletions internal/queries/queries.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 7 additions & 27 deletions internal/schema/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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{
Expand All @@ -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) {
Expand Down
32 changes: 32 additions & 0 deletions internal/schema/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")},
Expand Down
1 change: 1 addition & 0 deletions pkg/diff/sql_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -1718,6 +1718,7 @@ func (isg *indexSQLVertexGenerator) addDepsOnTableAddAlterIfNecessary(index sche
}
}

// Otherwise, we can drop the index whenever we want.
return nil
}

Expand Down

0 comments on commit 4fc0bff

Please sign in to comment.