Skip to content

Commit

Permalink
Support create/drop index with uppercase names (#356)
Browse files Browse the repository at this point in the history 
Fixes #355

Postgres stores index names with uppercase characters in the `pg_index`
catalog using the quoted version of the name. For example:

```
"idx_USERS_name"
```

whereas a lowercase index name would be stored as:

```
idx_users_name
```

This is different to how other object types are stored in their
respective catalogs. For example, table names are stored in
the`pg_class` catalog without quotes, regardless of whether they contain
uppercase characters.

This makes it necessary to strip quotes from index names when retrieving
them from the `pg_index` catalog when building the internal schema
representation.
  • Loading branch information
@andrew-farries
andrew-farries authored May 16, 2024
1 parent 4d3faeb commit a87fa36
Show file tree
Hide file tree
Showing 5 changed files with 190 additions and 83 deletions.
3 changes: 2 additions & 1 deletion pkg/migrations/op_create_index.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ func (o *OpCreateIndex) Complete(ctx context.Context, conn db.DB, tr SQLTransfor

func (o *OpCreateIndex) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer) error {
// drop the index concurrently
_, err := conn.ExecContext(ctx, fmt.Sprintf("DROP INDEX CONCURRENTLY IF EXISTS %s", o.Name))
_, err := conn.ExecContext(ctx, fmt.Sprintf("DROP INDEX CONCURRENTLY IF EXISTS %s",
pq.QuoteIdentifier(o.Name)))

return err
}
Expand Down
122 changes: 85 additions & 37 deletions pkg/migrations/op_create_index_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,52 +12,100 @@ import (
func TestCreateIndex(t *testing.T) {
t.Parallel()

ExecuteTests(t, TestCases{{
name: "create index",
migrations: []migrations.Migration{
{
Name: "01_add_table",
Operations: migrations.Operations{
&migrations.OpCreateTable{
Name: "users",
Columns: []migrations.Column{
{
Name: "id",
Type: "serial",
Pk: ptr(true),
},
{
Name: "name",
Type: "varchar(255)",
Nullable: ptr(false),
ExecuteTests(t, TestCases{
{
name: "create index",
migrations: []migrations.Migration{
{
Name: "01_add_table",
Operations: migrations.Operations{
&migrations.OpCreateTable{
Name: "users",
Columns: []migrations.Column{
{
Name: "id",
Type: "serial",
Pk: ptr(true),
},
{
Name: "name",
Type: "varchar(255)",
Nullable: ptr(false),
},
},
},
},
},
},
{
Name: "02_create_index",
Operations: migrations.Operations{
&migrations.OpCreateIndex{
Name: "idx_users_name",
Table: "users",
Columns: []string{"name"},
{
Name: "02_create_index",
Operations: migrations.Operations{
&migrations.OpCreateIndex{
Name: "idx_users_name",
Table: "users",
Columns: []string{"name"},
},
},
},
},
afterStart: func(t *testing.T, db *sql.DB, schema string) {
// The index has been created on the underlying table.
IndexMustExist(t, db, schema, "users", "idx_users_name")
},
afterRollback: func(t *testing.T, db *sql.DB, schema string) {
// The index has been dropped from the the underlying table.
IndexMustNotExist(t, db, schema, "users", "idx_users_name")
},
afterComplete: func(t *testing.T, db *sql.DB, schema string) {
// Complete is a no-op.
},
},
afterStart: func(t *testing.T, db *sql.DB, schema string) {
// The index has been created on the underlying table.
IndexMustExist(t, db, schema, "users", "idx_users_name")
},
afterRollback: func(t *testing.T, db *sql.DB, schema string) {
// The index has been dropped from the the underlying table.
IndexMustNotExist(t, db, schema, "users", "idx_users_name")
},
afterComplete: func(t *testing.T, db *sql.DB, schema string) {
// Complete is a no-op.
{
name: "create index with a mixed case name",
migrations: []migrations.Migration{
{
Name: "01_add_table",
Operations: migrations.Operations{
&migrations.OpCreateTable{
Name: "users",
Columns: []migrations.Column{
{
Name: "id",
Type: "serial",
Pk: ptr(true),
},
{
Name: "name",
Type: "varchar(255)",
Nullable: ptr(false),
},
},
},
},
},
{
Name: "02_create_index",
Operations: migrations.Operations{
&migrations.OpCreateIndex{
Name: "idx_USERS_name",
Table: "users",
Columns: []string{"name"},
},
},
},
},
afterStart: func(t *testing.T, db *sql.DB, schema string) {
// The index has been created on the underlying table.
IndexMustExist(t, db, schema, "users", "idx_USERS_name")
},
afterRollback: func(t *testing.T, db *sql.DB, schema string) {
// The index has been dropped from the the underlying table.
IndexMustNotExist(t, db, schema, "users", "idx_USERS_name")
},
afterComplete: func(t *testing.T, db *sql.DB, schema string) {
// Complete is a no-op.
},
},
}})
})
}

func TestCreateIndexOnMultipleColumns(t *testing.T) {
Expand Down
4 changes: 3 additions & 1 deletion pkg/migrations/op_drop_index.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"context"
"fmt"

"github.com/lib/pq"
"github.com/xataio/pgroll/pkg/db"
"github.com/xataio/pgroll/pkg/schema"
)
Expand All @@ -19,7 +20,8 @@ func (o *OpDropIndex) Start(ctx context.Context, conn db.DB, stateSchema string,

func (o *OpDropIndex) Complete(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error {
// drop the index concurrently
_, err := conn.ExecContext(ctx, fmt.Sprintf("DROP INDEX CONCURRENTLY IF EXISTS %s", o.Name))
_, err := conn.ExecContext(ctx, fmt.Sprintf("DROP INDEX CONCURRENTLY IF EXISTS %s",
pq.QuoteIdentifier(o.Name)))

return err
}
Expand Down
142 changes: 99 additions & 43 deletions pkg/migrations/op_drop_index_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,58 +12,114 @@ import (
func TestDropIndex(t *testing.T) {
t.Parallel()

ExecuteTests(t, TestCases{{
name: "drop index",
migrations: []migrations.Migration{
{
Name: "01_add_table",
Operations: migrations.Operations{
&migrations.OpCreateTable{
Name: "users",
Columns: []migrations.Column{
{
Name: "id",
Type: "serial",
Pk: ptr(true),
},
{
Name: "name",
Type: "varchar(255)",
Nullable: ptr(false),
ExecuteTests(t, TestCases{
{
name: "drop index",
migrations: []migrations.Migration{
{
Name: "01_add_table",
Operations: migrations.Operations{
&migrations.OpCreateTable{
Name: "users",
Columns: []migrations.Column{
{
Name: "id",
Type: "serial",
Pk: ptr(true),
},
{
Name: "name",
Type: "varchar(255)",
Nullable: ptr(false),
},
},
},
},
},
},
{
Name: "02_create_index",
Operations: migrations.Operations{
&migrations.OpCreateIndex{
Name: "idx_users_name",
Table: "users",
Columns: []string{"name"},
{
Name: "02_create_index",
Operations: migrations.Operations{
&migrations.OpCreateIndex{
Name: "idx_users_name",
Table: "users",
Columns: []string{"name"},
},
},
},
},
{
Name: "03_drop_index",
Operations: migrations.Operations{
&migrations.OpDropIndex{
Name: "idx_users_name",
{
Name: "03_drop_index",
Operations: migrations.Operations{
&migrations.OpDropIndex{
Name: "idx_users_name",
},
},
},
},
afterStart: func(t *testing.T, db *sql.DB, schema string) {
// The index has not yet been dropped.
IndexMustExist(t, db, schema, "users", "idx_users_name")
},
afterRollback: func(t *testing.T, db *sql.DB, schema string) {
// Rollback is a no-op.
},
afterComplete: func(t *testing.T, db *sql.DB, schema string) {
// The index has been removed from the underlying table.
IndexMustNotExist(t, db, schema, "users", "idx_users_name")
},
},
afterStart: func(t *testing.T, db *sql.DB, schema string) {
// The index has not yet been dropped.
IndexMustExist(t, db, schema, "users", "idx_users_name")
},
afterRollback: func(t *testing.T, db *sql.DB, schema string) {
// Rollback is a no-op.
},
afterComplete: func(t *testing.T, db *sql.DB, schema string) {
// The index has been removed from the underlying table.
IndexMustNotExist(t, db, schema, "users", "idx_users_name")
{
name: "drop index with a mixed case name",
migrations: []migrations.Migration{
{
Name: "01_add_table",
Operations: migrations.Operations{
&migrations.OpCreateTable{
Name: "users",
Columns: []migrations.Column{
{
Name: "id",
Type: "serial",
Pk: ptr(true),
},
{
Name: "name",
Type: "varchar(255)",
Nullable: ptr(false),
},
},
},
},
},
{
Name: "02_create_index",
Operations: migrations.Operations{
&migrations.OpCreateIndex{
Name: "idx_USERS_name",
Table: "users",
Columns: []string{"name"},
},
},
},
{
Name: "03_drop_index",
Operations: migrations.Operations{
&migrations.OpDropIndex{
Name: "idx_USERS_name",
},
},
},
},
afterStart: func(t *testing.T, db *sql.DB, schema string) {
// The index has not yet been dropped.
IndexMustExist(t, db, schema, "users", "idx_USERS_name")
},
afterRollback: func(t *testing.T, db *sql.DB, schema string) {
// Rollback is a no-op.
},
afterComplete: func(t *testing.T, db *sql.DB, schema string) {
// The index has been removed from the underlying table.
IndexMustNotExist(t, db, schema, "users", "idx_USERS_name")
},
},
}})
})
}
2 changes: 1 addition & 1 deletion pkg/state/state.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ BEGIN
)), '{}'::json)
FROM (
SELECT
reverse(split_part(reverse(pi.indexrelid::regclass::text), '.', 1)) as name,
replace(reverse(split_part(reverse(pi.indexrelid::regclass::text), '.', 1)), '"', '') as name,
pi.indisunique,
array_agg(a.attname) AS columns
FROM pg_index pi
Expand Down

0 comments on commit a87fa36

Please sign in to comment.