Skip to content

Commit

Permalink
Use original table name without prefix when processing create table (#…
Browse files Browse the repository at this point in the history
…572)

Using the original name, instead of calling `TemporaryName` which adds
the prefix `_pgroll_new_` in create table operations and also when
rolling back.
Includes necessary changes in tests.
Fixes: #571
  • Loading branch information
agedemenli authored Jan 8, 2025
1 parent 67f4a08 commit bd0fa1c
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 29 deletions.
2 changes: 1 addition & 1 deletion examples/29_set_replica_identity.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"table": "fruits",
"identity": {
"type": "index",
"index": "_pgroll_new_fruits_pkey"
"index": "fruits_pkey"
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/migrations/op_create_index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ func TestCreateIndexOnObjectsCreatedInSameMigration(t *testing.T) {
},
afterStart: func(t *testing.T, db *sql.DB, schema string) {
// The index has been created on the underlying table.
IndexMustExist(t, db, schema, migrations.TemporaryName("users"), "idx_users_name")
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.
Expand Down
22 changes: 8 additions & 14 deletions pkg/migrations/op_create_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,9 @@ func (o *OpCreateTable) Start(ctx context.Context, conn db.DB, latestSchema stri
return nil, fmt.Errorf("failed to create columns SQL: %w", err)
}

// Create the table under a temporary name
tempName := TemporaryName(o.Name)
// Create the table
_, err = conn.ExecContext(ctx, fmt.Sprintf("CREATE TABLE %s (%s)",
pq.QuoteIdentifier(tempName),
pq.QuoteIdentifier(o.Name),
columnsSQL))
if err != nil {
return nil, err
Expand All @@ -34,15 +33,15 @@ func (o *OpCreateTable) Start(ctx context.Context, conn db.DB, latestSchema stri
// Add comments to any columns that have them
for _, col := range o.Columns {
if col.Comment != nil {
if err := addCommentToColumn(ctx, conn, tempName, col.Name, col.Comment); err != nil {
if err := addCommentToColumn(ctx, conn, o.Name, col.Name, col.Comment); err != nil {
return nil, fmt.Errorf("failed to add comment to column: %w", err)
}
}
}

// Add comment to the table itself
if o.Comment != nil {
if err := addCommentToTable(ctx, conn, tempName, o.Comment); err != nil {
if err := addCommentToTable(ctx, conn, o.Name, o.Comment); err != nil {
return nil, fmt.Errorf("failed to add comment to table: %w", err)
}
}
Expand All @@ -54,18 +53,13 @@ func (o *OpCreateTable) Start(ctx context.Context, conn db.DB, latestSchema stri
}

func (o *OpCreateTable) Complete(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error {
tempName := TemporaryName(o.Name)
_, err := conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE IF EXISTS %s RENAME TO %s",
pq.QuoteIdentifier(tempName),
pq.QuoteIdentifier(o.Name)))
return err
// No-op
return nil
}

func (o *OpCreateTable) Rollback(ctx context.Context, conn db.DB, tr SQLTransformer, s *schema.Schema) error {
tempName := TemporaryName(o.Name)

_, err := conn.ExecContext(ctx, fmt.Sprintf("DROP TABLE IF EXISTS %s",
pq.QuoteIdentifier(tempName)))
pq.QuoteIdentifier(o.Name)))
return err
}

Expand Down Expand Up @@ -125,7 +119,7 @@ func (o *OpCreateTable) updateSchema(s *schema.Schema) *schema.Schema {
}
}
s.AddTable(o.Name, schema.Table{
Name: TemporaryName(o.Name),
Name: o.Name,
Columns: columns,
})

Expand Down
11 changes: 5 additions & 6 deletions pkg/migrations/op_create_table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func TestCreateTable(t *testing.T) {
},
afterStart: func(t *testing.T, db *sql.DB, schema string) {
// The foreign key constraint exists on the new table.
ValidatedForeignKeyMustExist(t, db, schema, migrations.TemporaryName("orders"), "fk_users_id")
ValidatedForeignKeyMustExist(t, db, schema, "orders", "fk_users_id")

// Inserting a row into the referenced table succeeds.
MustInsert(t, db, schema, "01_create_table", "users", map[string]string{
Expand Down Expand Up @@ -322,7 +322,7 @@ func TestCreateTable(t *testing.T) {
},
afterStart: func(t *testing.T, db *sql.DB, schema string) {
// The foreign key constraint exists on the new table.
ValidatedForeignKeyMustExist(t, db, schema, migrations.TemporaryName("orders"), "fk_users_id", withOnDeleteCascade())
ValidatedForeignKeyMustExist(t, db, schema, "orders", "fk_users_id", withOnDeleteCascade())

// Inserting a row into the referenced table succeeds.
MustInsert(t, db, schema, "01_create_table", "users", map[string]string{
Expand Down Expand Up @@ -412,7 +412,7 @@ func TestCreateTable(t *testing.T) {
},
afterStart: func(t *testing.T, db *sql.DB, schema string) {
// The check constraint exists on the new table.
CheckConstraintMustExist(t, db, schema, migrations.TemporaryName("users"), "check_name_length")
CheckConstraintMustExist(t, db, schema, "users", "check_name_length")

// Inserting a row into the table succeeds when the check constraint is satisfied.
MustInsert(t, db, schema, "01_create_table", "users", map[string]string{
Expand Down Expand Up @@ -469,11 +469,10 @@ func TestCreateTable(t *testing.T) {
},
},
afterStart: func(t *testing.T, db *sql.DB, schema string) {
tableName := migrations.TemporaryName("users")
// The comment has been added to the underlying table.
TableMustHaveComment(t, db, schema, tableName, "the users table")
TableMustHaveComment(t, db, schema, "users", "the users table")
// The comment has been added to the underlying column.
ColumnMustHaveComment(t, db, schema, tableName, "name", "the username")
ColumnMustHaveComment(t, db, schema, "users", "name", "the username")
},
afterRollback: func(t *testing.T, db *sql.DB, schema string) {
},
Expand Down
12 changes: 6 additions & 6 deletions pkg/migrations/op_drop_constraint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ func TestDropConstraint(t *testing.T) {
Operations: migrations.Operations{
&migrations.OpDropConstraint{
Table: "users",
Name: "_pgroll_new_users_name_key",
Name: "users_name_key",
Up: "name",
Down: "name || '-' || (random()*1000000)::integer",
},
Expand Down Expand Up @@ -438,7 +438,7 @@ func TestDropConstraint(t *testing.T) {
Operations: migrations.Operations{
&migrations.OpDropConstraint{
Table: "users",
Name: "_pgroll_new_users_name_key",
Name: "users_name_key",
Up: "name",
Down: "name || '-' || (random()*1000000)::integer",
},
Expand Down Expand Up @@ -537,7 +537,7 @@ func TestDropConstraint(t *testing.T) {
Operations: migrations.Operations{
&migrations.OpDropConstraint{
Table: "employees",
Name: "_pgroll_new_employees_department_id_key",
Name: "employees_department_id_key",
Up: "department_id",
Down: "department_id",
},
Expand Down Expand Up @@ -599,7 +599,7 @@ func TestDropConstraint(t *testing.T) {
Operations: migrations.Operations{
&migrations.OpDropConstraint{
Table: "posts",
Name: "_pgroll_new_posts_title_key",
Name: "posts_title_key",
Up: "title",
Down: "title",
},
Expand Down Expand Up @@ -742,7 +742,7 @@ func TestDropConstraint(t *testing.T) {
Operations: migrations.Operations{
&migrations.OpDropConstraint{
Table: "posts",
Name: "_pgroll_new_posts_title_key",
Name: "posts_title_key",
Up: "title",
Down: "title",
},
Expand Down Expand Up @@ -794,7 +794,7 @@ func TestDropConstraint(t *testing.T) {
Operations: migrations.Operations{
&migrations.OpDropConstraint{
Table: "posts",
Name: "_pgroll_new_posts_title_key",
Name: "posts_title_key",
Up: "title",
Down: "title",
},
Expand Down
2 changes: 1 addition & 1 deletion pkg/migrations/op_set_replica_identity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func TestSetReplicaIdentity(t *testing.T) {
Table: "users",
Identity: migrations.ReplicaIdentity{
Type: "index",
Index: "_pgroll_new_users_pkey",
Index: "users_pkey",
},
},
},
Expand Down

0 comments on commit bd0fa1c

Please sign in to comment.