diff --git a/README.md b/README.md
index d872d00..7a9578f 100644
--- a/README.md
+++ b/README.md
@@ -75,7 +75,7 @@ $ pg-schema-diff plan --dsn "postgres://postgres:postgres@localhost:5432/postgre
* The use of postgres native operations for zero-downtime migrations wherever possible:
* Concurrent index builds
* Online index replacement: If some index is changed, the new version will be built before the old version is dropped, preventing a window where no index is backing queries
- * Online check constraint builds: Check constraints are added as `INVALID` before being validated, eliminating the need
+ * Online constraint builds: Constraints (check, foreign key) are added as `INVALID` before being validated, eliminating the need
for a long access-exclusive lock on the table
* Online `NOT NULL` constraint creation using check constraints to eliminate the need for an access-exclusive lock on the table
* Prioritized index builds: Building new indexes is always prioritized over deleting old indexes
diff --git a/internal/migration_acceptance_tests/backwards_compat_cases_test.go b/internal/migration_acceptance_tests/backwards_compat_cases_test.go
index ed22c3b..81e76db 100644
--- a/internal/migration_acceptance_tests/backwards_compat_cases_test.go
+++ b/internal/migration_acceptance_tests/backwards_compat_cases_test.go
@@ -84,7 +84,6 @@ var backCompatAcceptanceTestCases = []acceptanceTestCase{
`,
},
expectedHazardTypes: []diff.MigrationHazardType{
- diff.MigrationHazardTypeAcquiresShareRowExclusiveLock,
diff.MigrationHazardTypeDeletesData,
},
diff --git a/internal/migration_acceptance_tests/database_schema_source_cases_test.go b/internal/migration_acceptance_tests/database_schema_source_cases_test.go
index 99120c2..cc1ddb9 100644
--- a/internal/migration_acceptance_tests/database_schema_source_cases_test.go
+++ b/internal/migration_acceptance_tests/database_schema_source_cases_test.go
@@ -117,7 +117,6 @@ var databaseSchemaSourceTestCases = []acceptanceTestCase{
`,
},
expectedHazardTypes: []diff.MigrationHazardType{
- diff.MigrationHazardTypeAcquiresShareRowExclusiveLock,
diff.MigrationHazardTypeDeletesData,
},
diff --git a/internal/migration_acceptance_tests/foreign_key_constraint_cases_test.go b/internal/migration_acceptance_tests/foreign_key_constraint_cases_test.go
index 16b2aef..887849d 100644
--- a/internal/migration_acceptance_tests/foreign_key_constraint_cases_test.go
+++ b/internal/migration_acceptance_tests/foreign_key_constraint_cases_test.go
@@ -20,7 +20,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
ON UPDATE SET NULL
NOT DEFERRABLE
);
- `,
+ `,
},
newSchemaDDL: []string{
`
@@ -37,13 +37,13 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
ON UPDATE SET NULL
NOT DEFERRABLE
);
- `,
+ `,
},
expectEmptyPlan: true,
},
{
- name: "Add FK with most options",
+ name: "Add FK",
oldSchemaDDL: []string{
`
CREATE TABLE "foo bar"(
@@ -54,7 +54,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
CREATE TABLE "foobar fk"(
fk_id INT
);
- `,
+ `,
},
newSchemaDDL: []string{
`
@@ -70,12 +70,137 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
ON UPDATE SET NULL
NOT DEFERRABLE
);
- `,
+ `,
+ },
+ },
+ {
+ name: "Add FK on partitioned",
+ oldSchemaDDL: []string{
+ `
+ CREATE TABLE "foo bar"(
+ id INT,
+ PRIMARY KEY (id)
+ );
+
+ CREATE TABLE "foobar fk"(
+ fk_id INT
+ ) PARTITION BY LIST (fk_id);
+ `,
+ },
+ newSchemaDDL: []string{
+ `
+ CREATE TABLE "foo bar"(
+ id INT,
+ PRIMARY KEY (id)
+ );
+
+ CREATE TABLE "foobar fk"(
+ fk_id INT,
+ FOREIGN KEY (fk_id) REFERENCES "foo bar"(id)
+ ON DELETE SET NULL
+ ON UPDATE SET NULL
+ NOT DEFERRABLE
+ ) PARTITION BY LIST (fk_id);
+ `,
},
expectedHazardTypes: []diff.MigrationHazardType{
diff.MigrationHazardTypeAcquiresShareRowExclusiveLock,
},
},
+ {
+ name: "Add FK on partition",
+ oldSchemaDDL: []string{
+ `
+ CREATE TABLE "foo bar"(
+ id INT,
+ PRIMARY KEY (id)
+ );
+
+ CREATE TABLE foobar_fk(
+ fk_id INT
+ ) PARTITION BY LIST (fk_id);
+ CREATE TABLE foobar_fk_1 PARTITION OF foobar_fk FOR VALUES IN (1);
+ `,
+ },
+ newSchemaDDL: []string{
+ `
+ CREATE TABLE "foo bar"(
+ id INT,
+ PRIMARY KEY (id)
+ );
+
+ CREATE TABLE foobar_fk(
+ fk_id INT
+ ) PARTITION BY LIST (fk_id);
+ CREATE TABLE foobar_fk_1 PARTITION OF foobar_fk FOR VALUES IN (1);
+ ALTER TABLE foobar_fk_1 ADD CONSTRAINT some_fk FOREIGN KEY (fk_id) REFERENCES "foo bar"(id);
+ `,
+ },
+ },
+ {
+ name: "Add FK referencing partitioned",
+ oldSchemaDDL: []string{
+ `
+ CREATE TABLE foobar(
+ id INT,
+ PRIMARY KEY (id)
+ ) PARTITION BY LIST (id);
+
+ CREATE TABLE "foobar fk"(
+ fk_id INT
+ );
+ `,
+ },
+ newSchemaDDL: []string{
+ `
+ CREATE TABLE foobar(
+ id INT,
+ PRIMARY KEY (id)
+ ) PARTITION BY LIST (id);
+
+ CREATE TABLE "foobar fk"(
+ fk_id INT,
+ FOREIGN KEY (fk_id) REFERENCES foobar(id)
+ ON DELETE SET NULL
+ ON UPDATE SET NULL
+ NOT DEFERRABLE
+ );
+ `,
+ },
+ },
+ {
+ name: "Add FK referencing partition",
+ oldSchemaDDL: []string{
+ `
+ CREATE TABLE foobar(
+ id INT,
+ PRIMARY KEY (id)
+ ) PARTITION BY LIST (id);
+ CREATE TABLE foobar_1 PARTITION OF foobar FOR VALUES IN (1);
+
+ CREATE TABLE "foobar fk"(
+ fk_id INT
+ );
+ `,
+ },
+ newSchemaDDL: []string{
+ `
+ CREATE TABLE foobar(
+ id INT,
+ PRIMARY KEY (id)
+ ) PARTITION BY LIST (id);
+ CREATE TABLE foobar_1 PARTITION OF foobar FOR VALUES IN (1);
+
+ CREATE TABLE "foobar fk"(
+ fk_id INT,
+ FOREIGN KEY (fk_id) REFERENCES foobar_1(id)
+ ON DELETE SET NULL
+ ON UPDATE SET NULL
+ NOT DEFERRABLE
+ );
+ `,
+ },
+ },
{
name: "Add FK (only referenced table is new)",
oldSchemaDDL: []string{
@@ -84,7 +209,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
id INT,
PRIMARY KEY (id)
);
- `,
+ `,
},
newSchemaDDL: []string{
`
@@ -97,10 +222,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
fk_id INT,
FOREIGN KEY (fk_id) REFERENCES foobar(id)
);
- `,
- },
- expectedHazardTypes: []diff.MigrationHazardType{
- diff.MigrationHazardTypeAcquiresShareRowExclusiveLock,
+ `,
},
},
{
@@ -110,7 +232,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
CREATE TABLE "foobar fk"(
fk_id INT
);
- `,
+ `,
},
newSchemaDDL: []string{
`
@@ -126,10 +248,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
ON UPDATE SET NULL
NOT DEFERRABLE
);
- `,
- },
- expectedHazardTypes: []diff.MigrationHazardType{
- diff.MigrationHazardTypeAcquiresShareRowExclusiveLock,
+ `,
},
},
{
@@ -148,7 +267,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
ON UPDATE SET NULL
NOT DEFERRABLE
);
- `,
+ `,
},
},
{
@@ -163,7 +282,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
CREATE TABLE "foobar fk"(
fk_id INT
);
- `,
+ `,
},
newSchemaDDL: []string{
`
@@ -178,9 +297,8 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
ALTER TABLE "foobar fk" ADD CONSTRAINT some_foobar_fk
FOREIGN KEY (fk_id) REFERENCES foobar(id)
NOT VALID;
- `,
+ `,
},
- expectedHazardTypes: []diff.MigrationHazardType{},
},
{
name: "Add FK (partitioned table)",
@@ -197,7 +315,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
fk_foo VARCHAR(255),
fk_id INT
);
- `,
+ `,
},
newSchemaDDL: []string{
`
@@ -213,10 +331,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
fk_id INT,
FOREIGN KEY (fk_foo, fk_id) REFERENCES foobar(foo, id)
);
- `,
- },
- expectedHazardTypes: []diff.MigrationHazardType{
- diff.MigrationHazardTypeAcquiresShareRowExclusiveLock,
+ `,
},
},
{
@@ -235,7 +350,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
ON UPDATE SET NULL
NOT DEFERRABLE
);
- `,
+ `,
},
newSchemaDDL: []string{
`
@@ -247,7 +362,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
CREATE TABLE "foobar fk"(
fk_id INT
);
- `,
+ `,
},
},
{
@@ -266,7 +381,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
ON UPDATE SET NULL
NOT DEFERRABLE
);
- `,
+ `,
},
newSchemaDDL: []string{
`
@@ -283,10 +398,10 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
ON UPDATE SET NULL
NOT DEFERRABLE
);
- `,
+ `,
},
expectedHazardTypes: []diff.MigrationHazardType{
- diff.MigrationHazardTypeAcquiresShareRowExclusiveLock,
+
diff.MigrationHazardTypeDeletesData,
},
},
@@ -307,7 +422,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
fk_id INT,
FOREIGN KEY (fk_foo, fk_id) REFERENCES schema_1.foobar(foo, id)
);
- `,
+ `,
},
newSchemaDDL: []string{
`
@@ -323,7 +438,37 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
fk_foo VARCHAR(255),
fk_id INT
);
- `,
+ `,
+ },
+ },
+ {
+ name: "Drop FK with table (references partitioned table)",
+ oldSchemaDDL: []string{
+ `
+ CREATE TABLE foobar(
+ foo varchar(255),
+ id INT,
+ PRIMARY KEY (id)
+ );
+
+ CREATE TABLE "foobar partitioned"(
+ foo varchar(255),
+ id INT,
+ PRIMARY KEY (foo, id)
+ ) PARTITION BY LIST (foo);
+ CREATE TABLE foobar_1 PARTITION OF "foobar partitioned" FOR VALUES IN ('1');
+ CREATE TABLE foobar_2 PARTITION OF "foobar partitioned" FOR VALUES IN ('2');
+
+ CREATE TABLE "foobar fk"(
+ fk_foo VARCHAR(255),
+ fk_id INT
+ );
+ ALTER TABLE "foobar fk" ADD CONSTRAINT some_fk
+ FOREIGN KEY (fk_foo, fk_id) REFERENCES "foobar partitioned"(foo, id);
+ `,
+ },
+ expectedHazardTypes: []diff.MigrationHazardType{
+ diff.MigrationHazardTypeDeletesData,
},
},
{
@@ -341,7 +486,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
ALTER TABLE "foobar fk" ADD CONSTRAINT some_fk
FOREIGN KEY (fk_id) REFERENCES foobar(id)
NOT VALID;
- `,
+ `,
},
newSchemaDDL: []string{
`
@@ -355,7 +500,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
);
ALTER TABLE "foobar fk" ADD CONSTRAINT some_fk
FOREIGN KEY (fk_id) REFERENCES foobar(id);
- `,
+ `,
},
expectedPlanDDL: []string{
"ALTER TABLE \"public\".\"foobar fk\" VALIDATE CONSTRAINT \"some_fk\"",
@@ -375,7 +520,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
);
ALTER TABLE "foobar fk" ADD CONSTRAINT some_fk
FOREIGN KEY (fk_id) REFERENCES foobar(id);
- `,
+ `,
},
newSchemaDDL: []string{
`
@@ -390,7 +535,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
ALTER TABLE "foobar fk" ADD CONSTRAINT some_fk
FOREIGN KEY (fk_id) REFERENCES foobar(id)
NOT VALID;
- `,
+ `,
},
},
{
@@ -411,7 +556,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
ALTER TABLE "foobar fk" ADD CONSTRAINT some_fk
FOREIGN KEY (fk_id) REFERENCES foobar(id)
NOT VALID;
- `,
+ `,
},
newSchemaDDL: []string{
`
@@ -428,10 +573,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
);
ALTER TABLE "foobar fk" ADD CONSTRAINT some_fk
FOREIGN KEY (fk_foo) REFERENCES foobar(foo);
- `,
- },
- expectedHazardTypes: []diff.MigrationHazardType{
- diff.MigrationHazardTypeAcquiresShareRowExclusiveLock,
+ `,
},
},
{
@@ -447,7 +589,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
fk_id INT,
FOREIGN KEY (fk_id) REFERENCES foobar(id)
);
- `,
+ `,
},
newSchemaDDL: []string{
`
@@ -461,10 +603,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
FOREIGN KEY (fk_id) REFERENCES foobar(id)
ON UPDATE CASCADE
);
- `,
- },
- expectedHazardTypes: []diff.MigrationHazardType{
- diff.MigrationHazardTypeAcquiresShareRowExclusiveLock,
+ `,
},
},
{
@@ -480,7 +619,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
fk_id INT,
FOREIGN KEY (fk_id) REFERENCES foobar(id)
);
- `,
+ `,
},
newSchemaDDL: []string{
`
@@ -494,10 +633,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
FOREIGN KEY (fk_id) REFERENCES foobar(id)
ON UPDATE CASCADE
);
- `,
- },
- expectedHazardTypes: []diff.MigrationHazardType{
- diff.MigrationHazardTypeAcquiresShareRowExclusiveLock,
+ `,
},
},
{
@@ -513,7 +649,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
fk_id INT,
FOREIGN KEY (fk_id) REFERENCES foobar(id)
);
- `,
+ `,
},
newSchemaDDL: []string{
`
@@ -526,7 +662,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
fk_id INT,
FOREIGN KEY (fk_id) REFERENCES foobar(id)
);
- `,
+ `,
},
expectedHazardTypes: []diff.MigrationHazardType{
diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
@@ -546,7 +682,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
fk_id BIGINT,
FOREIGN KEY (fk_id) REFERENCES foobar(id)
);
- `,
+ `,
},
newSchemaDDL: []string{
`
@@ -559,7 +695,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
fk_id TIMESTAMP,
FOREIGN KEY (fk_id) REFERENCES foobar(id)
);
- `,
+ `,
},
expectedPlanErrorContains: errValidatingPlan.Error(),
@@ -585,7 +721,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
) PARTITION BY LIST (foo);
CREATE TABLE foobar_1 PARTITION OF "foobar fk partitioned" FOR VALUES IN ('1');
CREATE TABLE foobar_2 PARTITION OF "foobar fk partitioned" FOR VALUES IN ('2');
- `,
+ `,
},
newSchemaDDL: []string{
`
@@ -606,12 +742,54 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
CREATE TABLE foobar_2 PARTITION OF "foobar fk partitioned" FOR VALUES IN ('2');
ALTER TABLE "foobar fk partitioned" ADD CONSTRAINT some_fk
FOREIGN KEY (fk_id) REFERENCES foobar(id);
- `,
+ `,
},
expectedHazardTypes: []diff.MigrationHazardType{
diff.MigrationHazardTypeAcquiresShareRowExclusiveLock,
},
},
+ {
+ name: "Switch FK owning table (from partitioned table)",
+ oldSchemaDDL: []string{
+ `
+ CREATE TABLE foobar(
+ id INT,
+ PRIMARY KEY (id)
+ );
+
+ CREATE TABLE "foobar fk"(
+ fk_id INT
+ );
+ CREATE TABLE "foobar fk partitioned"(
+ foo varchar(255),
+ fk_id INT
+ ) PARTITION BY LIST (foo);
+ ALTER TABLE "foobar fk partitioned" ADD CONSTRAINT some_fk
+ FOREIGN KEY (fk_id) REFERENCES foobar(id);
+ `,
+ },
+ newSchemaDDL: []string{
+ `
+ CREATE TABLE foobar(
+ id INT,
+ PRIMARY KEY (id)
+ );
+
+ CREATE TABLE "foobar fk"(
+ fk_id INT
+ );
+
+ CREATE TABLE "foobar fk partitioned"(
+ foo varchar(255),
+ fk_id INT
+ ) PARTITION BY LIST (foo);
+ CREATE TABLE foobar_1 PARTITION OF "foobar fk partitioned" FOR VALUES IN ('1');
+ CREATE TABLE foobar_2 PARTITION OF "foobar fk partitioned" FOR VALUES IN ('2');
+ ALTER TABLE "foobar fk" ADD CONSTRAINT some_fk
+ FOREIGN KEY (fk_id) REFERENCES foobar(id);
+ `,
+ },
+ },
{
name: "Switch FK owning table (analog tables in different schemas stay same)",
oldSchemaDDL: []string{
@@ -644,7 +822,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
);
ALTER TABLE schema_1."foobar fk" ADD CONSTRAINT some_fk
FOREIGN KEY (fk_id) REFERENCES schema_1.foobar(id);
- `,
+ `,
},
newSchemaDDL: []string{
`
@@ -681,8 +859,9 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
FOREIGN KEY (fk_id, fk_val) REFERENCES schema_1.foobar(id, val);
`},
expectedHazardTypes: []diff.MigrationHazardType{
- diff.MigrationHazardTypeAcquiresShareRowExclusiveLock,
+
diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
+ diff.MigrationHazardTypeAcquiresShareRowExclusiveLock,
diff.MigrationHazardTypeIndexBuild,
diff.MigrationHazardTypeIndexDropped,
},
@@ -711,7 +890,7 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
);
ALTER TABLE "foobar fk" ADD CONSTRAINT some_fk
FOREIGN KEY (fk_foo, fk_id) REFERENCES foobar(foo, id) NOT VALID ;
- `,
+ `,
},
newSchemaDDL: []string{
`
@@ -735,12 +914,11 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
);
ALTER TABLE "foobar fk" ADD CONSTRAINT some_fk
FOREIGN KEY (fk_foo, fk_id) REFERENCES "foobar partitioned"(foo, id);
- `,
+ `,
},
expectedHazardTypes: []diff.MigrationHazardType{
diff.MigrationHazardTypeIndexDropped,
diff.MigrationHazardTypeIndexBuild,
- diff.MigrationHazardTypeAcquiresShareRowExclusiveLock,
},
},
}
diff --git a/internal/migration_acceptance_tests/schema_cases_test.go b/internal/migration_acceptance_tests/schema_cases_test.go
index ed59b53..9a85cf3 100644
--- a/internal/migration_acceptance_tests/schema_cases_test.go
+++ b/internal/migration_acceptance_tests/schema_cases_test.go
@@ -337,7 +337,6 @@ var schemaAcceptanceTests = []acceptanceTestCase{
`,
},
expectedHazardTypes: []diff.MigrationHazardType{
- diff.MigrationHazardTypeAcquiresShareRowExclusiveLock,
diff.MigrationHazardTypeAuthzUpdate,
diff.MigrationHazardTypeDeletesData,
diff.MigrationHazardTypeHasUntrackableDependencies,
@@ -422,7 +421,6 @@ var schemaAcceptanceTests = []acceptanceTestCase{
`,
},
expectedHazardTypes: []diff.MigrationHazardType{
- diff.MigrationHazardTypeAcquiresShareRowExclusiveLock,
diff.MigrationHazardTypeDeletesData,
},
},
diff --git a/internal/migration_acceptance_tests/table_cases_test.go b/internal/migration_acceptance_tests/table_cases_test.go
index b97b064..8dc89bf 100644
--- a/internal/migration_acceptance_tests/table_cases_test.go
+++ b/internal/migration_acceptance_tests/table_cases_test.go
@@ -462,7 +462,6 @@ var tableAcceptanceTestCases = []acceptanceTestCase{
},
expectedHazardTypes: []diff.MigrationHazardType{
diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
- diff.MigrationHazardTypeAcquiresShareRowExclusiveLock,
diff.MigrationHazardTypeAuthzUpdate,
diff.MigrationHazardTypeImpactsDatabasePerformance,
diff.MigrationHazardTypeDeletesData,
@@ -531,7 +530,6 @@ var tableAcceptanceTestCases = []acceptanceTestCase{
},
expectedHazardTypes: []diff.MigrationHazardType{
diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
- diff.MigrationHazardTypeAcquiresShareRowExclusiveLock,
diff.MigrationHazardTypeAuthzUpdate,
diff.MigrationHazardTypeDeletesData,
diff.MigrationHazardTypeIndexDropped,
diff --git a/pkg/diff/sql_generator.go b/pkg/diff/sql_generator.go
index eeda9ad..be65a09 100644
--- a/pkg/diff/sql_generator.go
+++ b/pkg/diff/sql_generator.go
@@ -1,6 +1,7 @@
package diff
import (
+ "errors"
"fmt"
"regexp"
"sort"
@@ -245,37 +246,10 @@ func buildSchemaDiff(old, new schema.Schema) (schemaDiff, bool, error) {
if err != nil {
return schemaDiff{}, false, fmt.Errorf("diffing indexes: %w", err)
}
- foreignKeyConstraintDiffs, err := diffLists(old.ForeignKeyConstraints, new.ForeignKeyConstraints, func(old, new schema.ForeignKeyConstraint, _, _ int) (foreignKeyConstraintDiff, bool, error) {
- if _, isOnNewTable := addedTablesByName[new.OwningTable.GetName()]; isOnNewTable {
-
- // If the owning table is new, then it must be re-created (this occurs if the base table has been
- // re-created). In other words, a foreign key constraint must be re-created if the owning table or referenced
- // table is re-created
- return foreignKeyConstraintDiff{}, true, nil
- } else if _, isReferencingNewTable := addedTablesByName[new.ForeignTable.GetName()]; isReferencingNewTable {
- // Same as above, but for the referenced table
- return foreignKeyConstraintDiff{}, true, nil
- }
-
- // Set the new clone to be equal to the old for all fields that can actually be altered
- newClone := new
- if !old.IsValid && new.IsValid {
- // We only support alter from NOT VALID to VALID and no other alterations.
- // Instead of checking that each individual property is equal (that's a lot of parsing), we will just
- // assert that the constraint definitions are equal if we append "NOT VALID" to the new constraint def
- newClone.IsValid = old.IsValid
- newClone.ConstraintDef = fmt.Sprintf("%s NOT VALID", newClone.ConstraintDef)
- }
- if !cmp.Equal(old, newClone) {
- return foreignKeyConstraintDiff{}, true, nil
- }
- return foreignKeyConstraintDiff{
- oldAndNew[schema.ForeignKeyConstraint]{
- old: old,
- new: new,
- },
- }, false, nil
+ fsg := newForeignKeyConstraintSQLVertexGenerator(oldAndNew[schema.Schema]{old: old, new: new}, tableDiffs)
+ foreignKeyConstraintDiffs, err := diffLists(old.ForeignKeyConstraints, new.ForeignKeyConstraints, func(old, new schema.ForeignKeyConstraint, _, _ int) (foreignKeyConstraintDiff, bool, error) {
+ return buildForeignKeyConstraintDiff(fsg, addedTablesByName, old, new)
})
if err != nil {
return schemaDiff{}, false, fmt.Errorf("diffing foreign key constraints: %w", err)
@@ -526,15 +500,6 @@ func (schemaSQLGenerator) Alter(diff schemaDiff) ([]Statement, error) {
deletedTablesByName := buildSchemaObjByNameMap(diff.tableDiffs.deletes)
addedTablesByName := buildSchemaObjByNameMap(diff.tableDiffs.adds)
- indexesOldSchemaByTableName := make(map[string][]schema.Index)
- for _, idx := range diff.old.Indexes {
- indexesOldSchemaByTableName[idx.OwningTable.GetName()] = append(indexesOldSchemaByTableName[idx.OwningTable.GetName()], idx)
- }
- indexesInNewSchemaByTableName := make(map[string][]schema.Index)
- for _, idx := range diff.new.Indexes {
- indexesInNewSchemaByTableName[idx.OwningTable.GetName()] = append(indexesInNewSchemaByTableName[idx.OwningTable.GetName()], idx)
- }
-
namedSchemaStatements, err := diff.namedSchemaDiffs.resolveToSQLGroupedByEffect(&namedSchemaSQLGenerator{})
if err != nil {
return nil, fmt.Errorf("resolving named schema sql statements: %w", err)
@@ -559,7 +524,7 @@ func (schemaSQLGenerator) Alter(diff schemaDiff) ([]Statement, error) {
return nil, fmt.Errorf("resolving enum sql graphs: %w", err)
}
- attachPartitionSQLVertexGenerator := newAttachPartitionSQLVertexGenerator(indexesInNewSchemaByTableName, addedTablesByName)
+ attachPartitionSQLVertexGenerator := newAttachPartitionSQLVertexGenerator(diff.new.Indexes, diff.tableDiffs.adds)
attachPartitionGraphs, err := diff.tableDiffs.resolveToSQLGraph(attachPartitionSQLVertexGenerator)
if err != nil {
return nil, fmt.Errorf("resolving attach partition sql graphs: %w", err)
@@ -584,12 +549,7 @@ func (schemaSQLGenerator) Alter(diff schemaDiff) ([]Statement, error) {
return nil, fmt.Errorf("resolving index sql graphs: %w", err)
}
- fkConsGraphs, err := diff.foreignKeyConstraintDiffs.resolveToSQLGraph(&foreignKeyConstraintSQLVertexGenerator{
- deletedTablesByName: deletedTablesByName,
- addedTablesByName: addedTablesByName,
- indexInOldSchemaByTableName: indexesInNewSchemaByTableName,
- indexesInNewSchemaByTableName: indexesInNewSchemaByTableName,
- })
+ fkConsGraphs, err := diff.foreignKeyConstraintDiffs.resolveToSQLGraph(newForeignKeyConstraintSQLVertexGenerator(diff.oldAndNew, diff.tableDiffs))
if err != nil {
return nil, fmt.Errorf("resolving foreign key constraint sql graphs: %w", err)
}
@@ -667,6 +627,62 @@ func (schemaSQLGenerator) Alter(diff schemaDiff) ([]Statement, error) {
return statements, nil
}
+func buildIndexesByTableNameMap(indexes []schema.Index) map[string][]schema.Index {
+ var output = make(map[string][]schema.Index)
+ for _, idx := range indexes {
+ output[idx.OwningTable.GetName()] = append(output[idx.OwningTable.GetName()], idx)
+ }
+ return output
+}
+
+// buildChildrenByPartitionedIndexNameMap builds a map of indexes by their parent index name. This map will cross
+// all levels. As in, if a parent index has 5 children and each of those children has 5 children, the value for that
+// index wil contain 30 indexes (the 5 direct children adn the 25 leaves)
+func buildChildrenByPartitionedIndexNameMap(indexes []schema.Index) map[string][]schema.Index {
+ // Build a map of children by their direct parent index
+ var childrenByDirectParentIdxName = make(map[string][]schema.Index)
+ for _, idx := range indexes {
+ parentIdxName := ""
+ if idx.ParentIdx != nil {
+ parentIdxName = idx.ParentIdx.GetName()
+ }
+ childrenByDirectParentIdxName[parentIdxName] = append(childrenByDirectParentIdxName[parentIdxName], idx)
+ }
+
+ // Use this map to build a map of root index by child index name
+ var rootsByChildIdxName = make(map[string]schema.Index)
+ // We need to calculate this level by level. We will start with the roots.
+ var currentNodes = childrenByDirectParentIdxName[""]
+ for len(currentNodes) > 0 {
+ var idx = currentNodes[0]
+ currentNodes = currentNodes[1:]
+
+ rootIdx := idx
+ if idx.ParentIdx != nil {
+ rootIdx = rootsByChildIdxName[idx.ParentIdx.GetName()]
+ }
+ rootsByChildIdxName[idx.GetName()] = rootIdx
+
+ // We can now calculate the next level of children for this node
+ for _, child := range childrenByDirectParentIdxName[idx.GetName()] {
+ currentNodes = append(currentNodes, child)
+ }
+ }
+
+ // Build a map of all children by the root index name using the result of the previous step
+ var allChildrenByRootIndexName = make(map[string][]schema.Index)
+ for _, idx := range indexes {
+ if idx.ParentIdx == nil {
+ // Skip nodes without parents
+ continue
+ }
+ rootIdx := rootsByChildIdxName[idx.GetName()]
+ allChildrenByRootIndexName[rootIdx.GetName()] = append(allChildrenByRootIndexName[rootIdx.GetName()], idx)
+ }
+
+ return allChildrenByRootIndexName
+}
+
func buildSchemaObjByNameMap[S schema.Object](s []S) map[string]S {
return buildMap(s, func(s S) string {
return s.GetName()
@@ -1901,10 +1917,10 @@ type attachPartitionSQLVertexGenerator struct {
isPartitionAttachedAfterIdxBuildsByTableName map[string]bool
}
-func newAttachPartitionSQLVertexGenerator(indexesInNewSchemaByTableName map[string][]schema.Index, addedTablesByName map[string]schema.Table) *attachPartitionSQLVertexGenerator {
+func newAttachPartitionSQLVertexGenerator(newSchemaIndexes []schema.Index, addedTables []schema.Table) *attachPartitionSQLVertexGenerator {
return &attachPartitionSQLVertexGenerator{
- indexesInNewSchemaByTableName: indexesInNewSchemaByTableName,
- addedTablesByName: addedTablesByName,
+ indexesInNewSchemaByTableName: buildIndexesByTableNameMap(newSchemaIndexes),
+ addedTablesByName: buildSchemaObjByNameMap(addedTables),
isPartitionAttachedAfterIdxBuildsByTableName: make(map[string]bool),
}
@@ -1970,23 +1986,77 @@ func (a *attachPartitionSQLVertexGenerator) GetDeleteDependencies(_ schema.Table
return nil, nil
}
+func buildForeignKeyConstraintDiff(fsg *foreignKeyConstraintSQLVertexGenerator, addedTablesByName map[string]schema.Table, old, new schema.ForeignKeyConstraint) (foreignKeyConstraintDiff, bool, error) {
+ if _, isOnNewTable := addedTablesByName[new.OwningTable.GetName()]; isOnNewTable {
+ // If the owning table is new, then it must be re-created (this occurs if the base table has been
+ // re-created). In other words, a foreign key constraint must be re-created if the owning table or referenced
+ // table is re-created
+ return foreignKeyConstraintDiff{}, true, nil
+ }
+ if _, isReferencingNewTable := addedTablesByName[new.ForeignTable.GetName()]; isReferencingNewTable {
+ // Same as above, but for the referenced table
+ return foreignKeyConstraintDiff{}, true, nil
+ }
+ fkDiff := foreignKeyConstraintDiff{
+ oldAndNew[schema.ForeignKeyConstraint]{
+ old: old,
+ new: new,
+ },
+ }
+ if _, err := fsg.Alter(fkDiff); err != nil {
+ if errors.Is(err, ErrNotImplemented) {
+ // The foreign key must be re-created if the diff cannot be reconciled via alter statements
+ return foreignKeyConstraintDiff{}, true, nil
+ }
+ return foreignKeyConstraintDiff{}, false, fmt.Errorf("generating foreign key alter statements: %w", err)
+ }
+
+ return fkDiff, false, nil
+}
+
type foreignKeyConstraintSQLVertexGenerator struct {
- // deletedTablesByName is a map of table name to tables (and partitions) that are deleted
- // This is used to identify if the owning table is being dropped (meaning
- // the foreign key can be implicitly dropped)
- deletedTablesByName map[string]schema.Table
+ // newSchemaTablesByName is a map of table name to tables in the new schema.
+ newSchemaTablesByName map[string]schema.Table
// addedTablesByName is a map of table name to the added tables
// This is used to identify if hazards are necessary
addedTablesByName map[string]schema.Table
// indexInOldSchemaByTableName is a map of index name to the index in the old schema
// This is used to force the foreign key constraint to be dropped before the index it depends on is dropped
indexInOldSchemaByTableName map[string][]schema.Index
+ // childrenInOldSchemaByPartitionedIndexName gives all child indexes (across all levels) for a given
+ // partitioned index in the old schema.
+ childrenInOldSchemaByPartitionedIndexName map[string][]schema.Index
// indexesInNewSchemaByTableName is a map of index name to the index
// Same as above but for adds and after
indexesInNewSchemaByTableName map[string][]schema.Index
+ // childrenInNewSchemaByPartitionedIndexName gives all child indexes (across all levels) for a given
+ // partitioned index in the new schema.
+ childrenInNewSchemaByPartitionedIndexName map[string][]schema.Index
+}
+
+func newForeignKeyConstraintSQLVertexGenerator(oldAndNewSchema oldAndNew[schema.Schema], tableDiffs listDiff[schema.Table, tableDiff]) *foreignKeyConstraintSQLVertexGenerator {
+ return &foreignKeyConstraintSQLVertexGenerator{
+ newSchemaTablesByName: buildSchemaObjByNameMap(oldAndNewSchema.new.Tables),
+ addedTablesByName: buildSchemaObjByNameMap(tableDiffs.adds),
+ indexInOldSchemaByTableName: buildIndexesByTableNameMap(oldAndNewSchema.old.Indexes),
+ childrenInOldSchemaByPartitionedIndexName: buildChildrenByPartitionedIndexNameMap(oldAndNewSchema.old.Indexes),
+ indexesInNewSchemaByTableName: buildIndexesByTableNameMap(oldAndNewSchema.new.Indexes),
+ childrenInNewSchemaByPartitionedIndexName: buildChildrenByPartitionedIndexNameMap(oldAndNewSchema.new.Indexes),
+ }
}
func (f *foreignKeyConstraintSQLVertexGenerator) Add(con schema.ForeignKeyConstraint) ([]Statement, error) {
+ if con.IsValid {
+ table, ok := f.newSchemaTablesByName[con.OwningTable.GetName()]
+ if !ok {
+ return nil, fmt.Errorf("could not find table with name %s", con.OwningTable.GetName())
+ }
+ if !table.IsPartitioned() {
+ // Postgres does not support adding invalid foreign keys to partitioned tables.
+ return f.addAsInvalidThenValidateStatements(con), nil
+ }
+ }
+
var hazards []MigrationHazard
_, isOnNewTable := f.addedTablesByName[con.OwningTable.GetName()]
_, isReferencedTableNew := f.addedTablesByName[con.ForeignTable.GetName()]
@@ -2005,6 +2075,19 @@ func (f *foreignKeyConstraintSQLVertexGenerator) Add(con schema.ForeignKeyConstr
}}, nil
}
+func (f *foreignKeyConstraintSQLVertexGenerator) addAsInvalidThenValidateStatements(con schema.ForeignKeyConstraint) []Statement {
+ // If adding a valid constraint, we will first add the constraint as not valid then validate it in order to
+ // circumvent requiring a SHARE_ROW_EXCLUSIVE lock on the tables.
+ return []Statement{
+ {
+ DDL: fmt.Sprintf("%s %s NOT VALID", addConstraintPrefix(con.OwningTable, con.EscapedName), con.ConstraintDef),
+ Timeout: statementTimeoutDefault,
+ LockTimeout: lockTimeoutDefault,
+ },
+ validateConstraintStatement(con.OwningTable, con.EscapedName),
+ }
+}
+
func (f *foreignKeyConstraintSQLVertexGenerator) Delete(con schema.ForeignKeyConstraint) ([]Statement, error) {
// Always generate a drop statement even if the owning table is being deleted. This simplifies the logic a bit because
// if the owning table has a circular FK dependency with another table being dropped, we will need to explicitly drop
@@ -2052,6 +2135,10 @@ func (f *foreignKeyConstraintSQLVertexGenerator) GetAddAlterDependencies(con, _
// because of partitioned indexes, which are only valid when all child indexes have been built
for _, i := range f.indexesInNewSchemaByTableName[con.ForeignTable.GetName()] {
deps = append(deps, mustRun(f.GetSQLVertexId(con), diffTypeAddAlter).after(buildIndexVertexId(i.GetSchemaQualifiedName()), diffTypeAddAlter))
+ // Build a dependency on any child index if the index is partitioned
+ for _, c := range f.childrenInNewSchemaByPartitionedIndexName[i.GetName()] {
+ deps = append(deps, mustRun(f.GetSQLVertexId(con), diffTypeAddAlter).after(buildIndexVertexId(c.GetSchemaQualifiedName()), diffTypeAddAlter))
+ }
}
return deps, nil
@@ -2067,6 +2154,10 @@ func (f *foreignKeyConstraintSQLVertexGenerator) GetDeleteDependencies(con schem
// because of partitioned indexes, which are only valid when all child indexes have been built
for _, i := range f.indexInOldSchemaByTableName[con.ForeignTable.GetName()] {
deps = append(deps, mustRun(f.GetSQLVertexId(con), diffTypeDelete).before(buildIndexVertexId(i.GetSchemaQualifiedName()), diffTypeDelete))
+ // Build a dependency on any child index if the index is partitioned
+ for _, c := range f.childrenInOldSchemaByPartitionedIndexName[i.GetName()] {
+ deps = append(deps, mustRun(f.GetSQLVertexId(con), diffTypeDelete).before(buildIndexVertexId(c.GetSchemaQualifiedName()), diffTypeDelete))
+ }
}
return deps, nil
}
@@ -2220,7 +2311,7 @@ func (s sequenceOwnershipSQLVertexGenerator) Add(seq schema.Sequence) ([]Stateme
// If a new sequence has no owner, we don't need to alter it. The default is no owner
return nil, nil
}
- return []Statement{s.buildAlterOwnershipStmt(seq, nil)}, nil
+ return []Statement{s.buildAlterOwnershipStmt(seq)}, nil
}
func (s sequenceOwnershipSQLVertexGenerator) Delete(_ schema.Sequence) ([]Statement, error) {
@@ -2231,10 +2322,10 @@ func (s sequenceOwnershipSQLVertexGenerator) Alter(diff sequenceDiff) ([]Stateme
if cmp.Equal(diff.new.Owner, diff.old.Owner) {
return nil, nil
}
- return []Statement{s.buildAlterOwnershipStmt(diff.new, &diff.old)}, nil
+ return []Statement{s.buildAlterOwnershipStmt(diff.new)}, nil
}
-func (s sequenceOwnershipSQLVertexGenerator) buildAlterOwnershipStmt(new schema.Sequence, old *schema.Sequence) Statement {
+func (s sequenceOwnershipSQLVertexGenerator) buildAlterOwnershipStmt(new schema.Sequence) Statement {
newOwner := "NONE"
if new.Owner != nil {
newOwner = schema.FQEscapedColumnName(new.Owner.TableName, new.Owner.ColumnName)