Skip to content

Commit 77665f3

Browse files
kvchkvch
committed
address review notes
1 parent 5067e17 commit 77665f3

File tree

4 files changed

+42
-35
lines changed

4 files changed

+42
-35
lines changed

‎pkg/migrations/dbactions.go‎

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -844,12 +844,12 @@ func (a *setReplicaIdentityAction) Execute(ctx context.Context) error {
844844

845845
type alterReferencesAction struct {
846846
conn db.DB
847-
referencedBy schema.ReferencedBy
847+
referencedBy map[string][]*schema.ReferencedBy
848848
table string
849849
column string
850850
}
851851

852-
func NewAlterReferencesAction(conn db.DB, referencedBy schema.ReferencedBy, table, column string) *alterReferencesAction {
852+
func NewAlterReferencesAction(conn db.DB, referencedBy map[string][]*schema.ReferencedBy, table, column string) *alterReferencesAction {
853853
return &alterReferencesAction{
854854
conn: conn,
855855
referencedBy: referencedBy,
@@ -859,30 +859,30 @@ func NewAlterReferencesAction(conn db.DB, referencedBy schema.ReferencedBy, tabl
859859
}
860860

861861
func (a *alterReferencesAction) Execute(ctx context.Context) error {
862-
//for table, constraints := range a.referencedBy {
863-
// for _, constraint := range constraints {
864-
// // Drop the existing constraint
865-
// _, err := a.conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s DROP CONSTRAINT %s",
866-
// pq.QuoteIdentifier(table),
867-
// pq.QuoteIdentifier(constraint.Name),
868-
// ))
869-
// if err != nil {
870-
// return fmt.Errorf("dropping constraint %s on %s: %w", constraint.Name, table, err)
871-
// }
872-
873-
// // Recreate the constraint with the table and new column
874-
// newDef := strings.ReplaceAll(constraint.Definition, a.column, pq.QuoteIdentifier(TemporaryName(a.column)))
875-
// newDef = strings.ReplaceAll(newDef, a.table, pq.QuoteIdentifier(a.table))
876-
// _, err = a.conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s ADD CONSTRAINT %s %s",
877-
// pq.QuoteIdentifier(table),
878-
// pq.QuoteIdentifier(constraint.Name),
879-
// newDef,
880-
// ))
881-
// if err != nil {
882-
// return fmt.Errorf("altering references for %s.%s: %w", a.table, a.column, err)
883-
// }
884-
885-
// }
886-
//}
862+
for table, constraints := range a.referencedBy {
863+
for _, constraint := range constraints {
864+
// Drop the existing constraint
865+
_, err := a.conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s DROP CONSTRAINT %s",
866+
pq.QuoteIdentifier(table),
867+
pq.QuoteIdentifier(constraint.Name),
868+
))
869+
if err != nil {
870+
return fmt.Errorf("dropping constraint %s on %s: %w", constraint.Name, table, err)
871+
}
872+
873+
// Recreate the constraint with the table and new column
874+
newDef := strings.ReplaceAll(constraint.Definition, a.column, pq.QuoteIdentifier(TemporaryName(a.column)))
875+
newDef = strings.ReplaceAll(newDef, a.table, pq.QuoteIdentifier(a.table))
876+
_, err = a.conn.ExecContext(ctx, fmt.Sprintf("ALTER TABLE %s ADD CONSTRAINT %s %s",
877+
pq.QuoteIdentifier(table),
878+
pq.QuoteIdentifier(constraint.Name),
879+
newDef,
880+
))
881+
if err != nil {
882+
return fmt.Errorf("altering references for %s.%s: %w", a.table, a.column, err)
883+
}
884+
885+
}
886+
}
887887
return nil
888888
}

‎pkg/migrations/op_alter_column_test.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -732,6 +732,13 @@ func TestAlterPrimaryKeyColumns(t *testing.T) {
732732
Up: "CAST(id AS bigint)",
733733
Down: "SELECT CASE WHEN id < 2147483647 THEN CAST(id AS int) ELSE 0 END",
734734
},
735+
&migrations.OpAlterColumn{
736+
Table: "people",
737+
Column: "manages",
738+
Type: ptr("bigint"),
739+
Up: "CAST(manages AS bigint)",
740+
Down: "SELECT CASE WHEN manages < 2147483647 THEN CAST(manages AS int) ELSE 0 END",
741+
},
735742
},
736743
},
737744
},
@@ -762,6 +769,8 @@ func TestAlterPrimaryKeyColumns(t *testing.T) {
762769
"id": bigint,
763770
"name": "pgroll v2 release party",
764771
})
772+
773+
// Inserting a row with a bigint value into the new column should succeed
765774
MustInsert(t, db, schema, "02_alter_column", "people", map[string]string{
766775
"id": "2",
767776
"name": "bob",

‎pkg/schema/schema.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,8 @@ type Table struct {
6363
ExcludeConstraints map[string]*ExcludeConstraint `json:"excludeConstraints"`
6464

6565
// ReferencedBy is a map of table names that reference this table by foreign key
66-
// The key is the name of the referencing table, and the value is the name and
67-
// the definition of the foreign key constraint
68-
//ReferencedBy map[string][]*ReferencedBy `json:"referencedBy"`
69-
ReferencedBy ReferencedBy `json:"referencedBy"`
66+
// The key is the name of the referencing table, and the value is a slice of foreign key references
67+
ReferencedBy map[string][]*ReferencedBy `json:"referencedBy"`
7068

7169
// Whether or not the table has been deleted in the virtual schema
7270
Deleted bool `json:"-"`

‎pkg/state/init.sql‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -348,17 +348,17 @@ BEGIN
348348
AND ref_attr.attnum = ANY (fk_info.confkey) -- join the columns of the referenced table
349349
GROUP BY fk_info.conname, fk_info.conrelid, fk_info.columns, fk_info.confrelid, fk_info.confmatchtype, fk_info.confdeltype, fk_info.confupdtype, fk_info.relname) AS fk_details), 'referencedBy', (
350350
SELECT
351-
json_object_agg(ref_fk.conname, json_build_object('name', ref_fk.conname, 'table', ref_fk.relname, 'definition', ref_fk.definition))
351+
json_object_agg(ref_table, ref_constraints)
352352
FROM (
353353
SELECT
354-
ref_constraint.conname,
355-
ref_cl.relname,
356-
pg_get_constraintdef(ref_constraint.oid) AS definition
354+
ref_cl.relname AS ref_table,
355+
json_agg(json_build_object('name', ref_constraint.conname, 'table', ref_cl.relname, 'definition', pg_get_constraintdef(ref_constraint.oid))) AS ref_constraints
357356
FROM pg_constraint AS ref_constraint
358357
INNER JOIN pg_class ref_cl ON ref_constraint.conrelid = ref_cl.oid
359358
WHERE
360359
ref_constraint.confrelid = t.oid
361360
AND ref_constraint.contype = 'f'
361+
GROUP BY ref_cl.relname
362362
) AS ref_fk)))), '{}'::json)
363363
FROM pg_class AS t
364364
INNER JOIN pg_namespace AS ns ON t.relnamespace = ns.oid

0 commit comments

Comments
 (0)