From 6b98d2a4de3dc92ca8669b75631579d3f5a39445 Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Wed, 11 Nov 2026 22:18:20 +0000 Subject: [PATCH REL_17 v3 1/3] Check partition persistence on both sides of foreign keys The foreign key persistence checks are applied only to the relations named in the constraint. Unlogged partitioned tables may have permanent partitions and vice versa, and neither the recursion when the constraint is created, nor the cloning performed when a partition is created or attached, nor SET LOGGED/UNLOGGED looks at the partitions. A permanent partition could therefore reference rows in an unlogged table or an unlogged partition, for example: CREATE UNLOGGED TABLE pk (a int PRIMARY KEY); CREATE UNLOGGED TABLE fk (x int) PARTITION BY RANGE (x); CREATE TABLE fk1 PARTITION OF fk FOR VALUES FROM (0) TO (100); ALTER TABLE fk ADD FOREIGN KEY (x) REFERENCES pk; After a crash, recovery resets the unlogged relation to its init fork: the referenced rows vanish while the referencing rows remain, leaving the constraint violated, and a dump of the database can no longer be restored. Fix this by rejecting any foreign key that connects a permanent relation with storage in the referencing partition tree with an unlogged relation with storage in the referenced partition tree, when the constraint is created, when it is cloned for a new partition on either side, and when a partition changes persistence. The persistence of partitioned tables themselves is not considered here, as they hold no data; the existing checks on the named relations are kept as they are. --- src/backend/commands/tablecmds.c | 101 +++++++++++++++++++++- src/test/regress/expected/foreign_key.out | 101 ++++++++++++++++++++++ src/test/regress/sql/foreign_key.sql | 74 ++++++++++++++++ 3 files changed, 274 insertions(+), 2 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 982d98d43fa..88c0402c847 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -556,6 +556,9 @@ static void addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, static void CloneForeignKeyConstraints(List **wqueue, Relation parentRel, Relation partitionRel); static void CloneFkReferenced(Relation parentRel, Relation partitionRel); +static Oid FindStoredRelWithPersistence(Oid relid, char persistence, + Oid skipRelid); +static void CheckFkPersistenceTrees(Oid fkrelid, Oid pkrelid); static void CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel); static void createForeignKeyCheckTriggers(Oid myRelOid, Oid refRelOid, @@ -10338,6 +10341,13 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, break; } + /* + * The checks above only look at the named relations. If either side is + * partitioned, its partitions can have a different persistence, so check + * those too. + */ + CheckFkPersistenceTrees(RelationGetRelid(rel), RelationGetRelid(pkrel)); + /* * Look up the referencing attributes to make sure they exist, and record * their attnums and type OIDs. @@ -11251,6 +11261,69 @@ CloneForeignKeyConstraints(List **wqueue, Relation parentRel, CloneFkReferenced(parentRel, partitionRel); } +/* + * Return a relation with storage and the given persistence in the partition + * tree rooted at relid, other than skipRelid, or InvalidOid if there is none. + * If relid is not a partitioned table, only relid itself is considered. + * + * Partitioned tables are skipped: they hold no data, so their persistence + * does not matter for what is left after crash recovery. + */ +static Oid +FindStoredRelWithPersistence(Oid relid, char persistence, Oid skipRelid) +{ + List *rels; + ListCell *lc; + + if (get_rel_relkind(relid) == RELKIND_PARTITIONED_TABLE) + rels = find_all_inheritors(relid, AccessShareLock, NULL); + else + rels = list_make1_oid(relid); + + foreach(lc, rels) + { + Oid childid = lfirst_oid(lc); + + if (childid == skipRelid || + !RELKIND_HAS_STORAGE(get_rel_relkind(childid))) + continue; + if (get_rel_persistence(childid) == persistence) + return childid; + } + + return InvalidOid; +} + +/* + * Check that a foreign key from fkrelid to pkrelid does not make a permanent + * table reference an unlogged one, taking all partitions of both sides into + * account. An unlogged partitioned table may have permanent partitions and + * vice versa, so the persistence of the named relations is not enough. + */ +static void +CheckFkPersistenceTrees(Oid fkrelid, Oid pkrelid) +{ + Oid unloggedRelid; + Oid permRelid; + + unloggedRelid = FindStoredRelWithPersistence(pkrelid, + RELPERSISTENCE_UNLOGGED, + InvalidOid); + if (!OidIsValid(unloggedRelid)) + return; + + permRelid = FindStoredRelWithPersistence(fkrelid, + RELPERSISTENCE_PERMANENT, + InvalidOid); + if (OidIsValid(permRelid)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_TABLE_DEFINITION), + errmsg("constraints on permanent tables may reference only permanent tables"), + errdetail("Permanent table \"%s\" would reference unlogged table \"%s\".", + get_rel_name(permRelid), + get_rel_name(unloggedRelid)))); +} + /* * CloneFkReferenced * Subroutine for CloneForeignKeyConstraints @@ -11352,6 +11425,9 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) /* We need the same lock level that CreateTrigger will acquire */ fkRel = table_open(constrForm->conrelid, ShareRowExclusiveLock); + CheckFkPersistenceTrees(constrForm->conrelid, + RelationGetRelid(partitionRel)); + indexOid = constrForm->conindid; DeconstructFkConstraintRow(tuple, &numfks, @@ -11568,6 +11644,9 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) (void) find_all_inheritors(RelationGetRelid(pkrel), ShareRowExclusiveLock, NULL); + CheckFkPersistenceTrees(RelationGetRelid(partRel), + constrForm->confrelid); + DeconstructFkConstraintRow(tuple, &numfks, conkey, confkey, conpfeqop, conppeqop, conffeqop, &numfkdelsetcols, confdelsetcols); @@ -17867,6 +17946,7 @@ ATPrepChangePersistence(Relation rel, bool toLogged) { Oid foreignrelid; Relation foreignrel; + Oid otherrelid; /* the opposite end of what we used as scankey */ foreignrelid = toLogged ? con->confrelid : con->conrelid; @@ -17877,24 +17957,41 @@ ATPrepChangePersistence(Relation rel, bool toLogged) foreignrel = relation_open(foreignrelid, AccessShareLock); + /* + * If the relation has storage, also check the partitions on the + * other side, as they can differ from their parent. Skip the + * relation itself, in case it is part of that partition tree. + */ + if (RELKIND_HAS_STORAGE(rel->rd_rel->relkind)) + otherrelid = + FindStoredRelWithPersistence(foreignrelid, + toLogged ? RELPERSISTENCE_UNLOGGED : RELPERSISTENCE_PERMANENT, + RelationGetRelid(rel)); + else + otherrelid = InvalidOid; + if (toLogged) { if (!RelationIsPermanent(foreignrel)) + otherrelid = foreignrelid; + if (OidIsValid(otherrelid)) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("could not change table \"%s\" to logged because it references unlogged table \"%s\"", RelationGetRelationName(rel), - RelationGetRelationName(foreignrel)), + get_rel_name(otherrelid)), errtableconstraint(rel, NameStr(con->conname)))); } else { if (RelationIsPermanent(foreignrel)) + otherrelid = foreignrelid; + if (OidIsValid(otherrelid)) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("could not change table \"%s\" to unlogged because it references logged table \"%s\"", RelationGetRelationName(rel), - RelationGetRelationName(foreignrel)), + get_rel_name(otherrelid)), errtableconstraint(rel, NameStr(con->conname)))); } diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 835dbb13301..314155fd3c3 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -3310,3 +3310,104 @@ DROP CAST (fk_err_vch AS int); DROP FUNCTION fk_err_vcast(fk_err_vch); DROP TYPE fk_err_vch; DROP ROLE regress_fk_err_role; +-- FK persistence rules apply to the partitions on both sides, not just to +-- the named tables: unlogged partitioned tables can have permanent +-- partitions and vice versa, and crash recovery resets every unlogged +-- partition, leaving rows in permanent referencing partitions dangling. +CREATE SCHEMA fkpart14; +SET search_path TO fkpart14; +-- referenced side +CREATE TABLE pk (a int PRIMARY KEY) PARTITION BY RANGE (a); +CREATE UNLOGGED TABLE pk1 PARTITION OF pk FOR VALUES FROM (0) TO (100); +CREATE TABLE fk (x int REFERENCES pk); -- fails +ERROR: constraints on permanent tables may reference only permanent tables +DETAIL: Permanent table "fk" would reference unlogged table "pk1". +CREATE UNLOGGED TABLE ufk (x int REFERENCES pk); +ALTER TABLE ufk SET LOGGED; -- fails +ERROR: could not change table "ufk" to logged because it references unlogged table "pk1" +DROP TABLE ufk, pk; +CREATE TABLE pk (a int PRIMARY KEY) PARTITION BY RANGE (a); +CREATE TABLE fk (x int REFERENCES pk); +CREATE UNLOGGED TABLE pk1 PARTITION OF pk FOR VALUES FROM (0) TO (100); -- fails +ERROR: constraints on permanent tables may reference only permanent tables +DETAIL: Permanent table "fk" would reference unlogged table "pk1". +CREATE UNLOGGED TABLE pk2 (a int PRIMARY KEY); +ALTER TABLE pk ATTACH PARTITION pk2 FOR VALUES FROM (0) TO (100); -- fails +ERROR: constraints on permanent tables may reference only permanent tables +DETAIL: Permanent table "fk" would reference unlogged table "pk2". +CREATE TABLE pk3 (a int PRIMARY KEY) PARTITION BY RANGE (a); +CREATE UNLOGGED TABLE pk31 PARTITION OF pk3 FOR VALUES FROM (0) TO (50); +ALTER TABLE pk ATTACH PARTITION pk3 FOR VALUES FROM (0) TO (100); -- fails +ERROR: constraints on permanent tables may reference only permanent tables +DETAIL: Permanent table "fk" would reference unlogged table "pk31". +-- ok, only the persistence of partitions with storage matters +CREATE UNLOGGED TABLE pk4 (a int PRIMARY KEY) PARTITION BY RANGE (a); +CREATE TABLE pk41 PARTITION OF pk4 FOR VALUES FROM (0) TO (50); +ALTER TABLE pk ATTACH PARTITION pk4 FOR VALUES FROM (0) TO (100); +ALTER TABLE pk41 SET UNLOGGED; -- fails +ERROR: could not change table "pk41" to unlogged because it references logged table "fk" +DROP TABLE fk, pk, pk2, pk3; +-- referencing side +CREATE UNLOGGED TABLE pk (a int PRIMARY KEY); +CREATE UNLOGGED TABLE fk (x int) PARTITION BY RANGE (x); +CREATE TABLE fk1 PARTITION OF fk FOR VALUES FROM (0) TO (100); +ALTER TABLE fk ADD FOREIGN KEY (x) REFERENCES pk; -- fails +ERROR: constraints on permanent tables may reference only permanent tables +DETAIL: Permanent table "fk1" would reference unlogged table "pk". +DROP TABLE fk1; +CREATE UNLOGGED TABLE fk1 PARTITION OF fk FOR VALUES FROM (0) TO (100); +ALTER TABLE fk ADD FOREIGN KEY (x) REFERENCES pk; +CREATE TABLE fk2 PARTITION OF fk FOR VALUES FROM (100) TO (200); -- fails +ERROR: constraints on permanent tables may reference only permanent tables +DETAIL: Permanent table "fk2" would reference unlogged table "pk". +CREATE TABLE fk3 (x int); +ALTER TABLE fk ATTACH PARTITION fk3 FOR VALUES FROM (100) TO (200); -- fails +ERROR: constraints on permanent tables may reference only permanent tables +DETAIL: Permanent table "fk3" would reference unlogged table "pk". +CREATE UNLOGGED TABLE fk4 (x int) PARTITION BY RANGE (x); +CREATE TABLE fk41 PARTITION OF fk4 FOR VALUES FROM (100) TO (150); +ALTER TABLE fk ATTACH PARTITION fk4 FOR VALUES FROM (100) TO (200); -- fails +ERROR: constraints on permanent tables may reference only permanent tables +DETAIL: Permanent table "fk41" would reference unlogged table "pk". +ALTER TABLE fk1 SET LOGGED; -- fails +ERROR: could not change table "fk1" to logged because it references unlogged table "pk" +DROP TABLE fk, fk3, fk4, pk; +-- both sides partitioned +CREATE TABLE pk (a int PRIMARY KEY) PARTITION BY RANGE (a); +CREATE TABLE pk1 PARTITION OF pk FOR VALUES FROM (0) TO (100); +CREATE UNLOGGED TABLE fk (x int REFERENCES pk) PARTITION BY RANGE (x); +CREATE UNLOGGED TABLE fk1 PARTITION OF fk FOR VALUES FROM (0) TO (100); +CREATE UNLOGGED TABLE pk2 PARTITION OF pk FOR VALUES FROM (100) TO (200); +CREATE TABLE fk2 PARTITION OF fk FOR VALUES FROM (100) TO (200); -- fails +ERROR: constraints on permanent tables may reference only permanent tables +DETAIL: Permanent table "fk2" would reference unlogged table "pk2". +ALTER TABLE fk1 SET LOGGED; -- fails +ERROR: could not change table "fk1" to logged because it references unlogged table "pk2" +ALTER TABLE pk DETACH PARTITION pk2; +DROP TABLE pk2; +CREATE TABLE fk2 PARTITION OF fk FOR VALUES FROM (100) TO (200); +CREATE UNLOGGED TABLE pk2 PARTITION OF pk FOR VALUES FROM (100) TO (200); -- fails +ERROR: constraints on permanent tables may reference only permanent tables +DETAIL: Permanent table "fk2" would reference unlogged table "pk2". +ALTER TABLE pk1 SET UNLOGGED; -- fails +ERROR: could not change table "pk1" to unlogged because it references logged table "fk2" +-- ok, unlogged partitions of the referencing side may still be added +CREATE UNLOGGED TABLE fk3 PARTITION OF fk FOR VALUES FROM (200) TO (300); +DROP TABLE fk, pk; +-- self-referencing +CREATE TABLE self (a int PRIMARY KEY, b int REFERENCES self) PARTITION BY RANGE (a); +CREATE TABLE self1 PARTITION OF self FOR VALUES FROM (0) TO (100); +CREATE UNLOGGED TABLE self2 PARTITION OF self FOR VALUES FROM (100) TO (200); -- fails +ERROR: constraints on permanent tables may reference only permanent tables +DETAIL: Permanent table "self1" would reference unlogged table "self2". +CREATE TABLE self2 PARTITION OF self FOR VALUES FROM (100) TO (200); +ALTER TABLE self2 SET UNLOGGED; -- fails +ERROR: could not change table "self2" to unlogged because it references logged table "self" +DROP TABLE self; +-- ok, the table being changed is not counted twice +CREATE TABLE self (a int PRIMARY KEY, b int REFERENCES self) PARTITION BY RANGE (a); +CREATE UNLOGGED TABLE self1 PARTITION OF self FOR VALUES FROM (0) TO (100); +ALTER TABLE self1 SET LOGGED; +DROP TABLE self; +DROP SCHEMA fkpart14; +RESET search_path; diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index 1da88b2aa0b..eb1875b0908 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -2381,3 +2381,77 @@ DROP CAST (fk_err_vch AS int); DROP FUNCTION fk_err_vcast(fk_err_vch); DROP TYPE fk_err_vch; DROP ROLE regress_fk_err_role; + +-- FK persistence rules apply to the partitions on both sides, not just to +-- the named tables: unlogged partitioned tables can have permanent +-- partitions and vice versa, and crash recovery resets every unlogged +-- partition, leaving rows in permanent referencing partitions dangling. +CREATE SCHEMA fkpart14; +SET search_path TO fkpart14; +-- referenced side +CREATE TABLE pk (a int PRIMARY KEY) PARTITION BY RANGE (a); +CREATE UNLOGGED TABLE pk1 PARTITION OF pk FOR VALUES FROM (0) TO (100); +CREATE TABLE fk (x int REFERENCES pk); -- fails +CREATE UNLOGGED TABLE ufk (x int REFERENCES pk); +ALTER TABLE ufk SET LOGGED; -- fails +DROP TABLE ufk, pk; +CREATE TABLE pk (a int PRIMARY KEY) PARTITION BY RANGE (a); +CREATE TABLE fk (x int REFERENCES pk); +CREATE UNLOGGED TABLE pk1 PARTITION OF pk FOR VALUES FROM (0) TO (100); -- fails +CREATE UNLOGGED TABLE pk2 (a int PRIMARY KEY); +ALTER TABLE pk ATTACH PARTITION pk2 FOR VALUES FROM (0) TO (100); -- fails +CREATE TABLE pk3 (a int PRIMARY KEY) PARTITION BY RANGE (a); +CREATE UNLOGGED TABLE pk31 PARTITION OF pk3 FOR VALUES FROM (0) TO (50); +ALTER TABLE pk ATTACH PARTITION pk3 FOR VALUES FROM (0) TO (100); -- fails +-- ok, only the persistence of partitions with storage matters +CREATE UNLOGGED TABLE pk4 (a int PRIMARY KEY) PARTITION BY RANGE (a); +CREATE TABLE pk41 PARTITION OF pk4 FOR VALUES FROM (0) TO (50); +ALTER TABLE pk ATTACH PARTITION pk4 FOR VALUES FROM (0) TO (100); +ALTER TABLE pk41 SET UNLOGGED; -- fails +DROP TABLE fk, pk, pk2, pk3; +-- referencing side +CREATE UNLOGGED TABLE pk (a int PRIMARY KEY); +CREATE UNLOGGED TABLE fk (x int) PARTITION BY RANGE (x); +CREATE TABLE fk1 PARTITION OF fk FOR VALUES FROM (0) TO (100); +ALTER TABLE fk ADD FOREIGN KEY (x) REFERENCES pk; -- fails +DROP TABLE fk1; +CREATE UNLOGGED TABLE fk1 PARTITION OF fk FOR VALUES FROM (0) TO (100); +ALTER TABLE fk ADD FOREIGN KEY (x) REFERENCES pk; +CREATE TABLE fk2 PARTITION OF fk FOR VALUES FROM (100) TO (200); -- fails +CREATE TABLE fk3 (x int); +ALTER TABLE fk ATTACH PARTITION fk3 FOR VALUES FROM (100) TO (200); -- fails +CREATE UNLOGGED TABLE fk4 (x int) PARTITION BY RANGE (x); +CREATE TABLE fk41 PARTITION OF fk4 FOR VALUES FROM (100) TO (150); +ALTER TABLE fk ATTACH PARTITION fk4 FOR VALUES FROM (100) TO (200); -- fails +ALTER TABLE fk1 SET LOGGED; -- fails +DROP TABLE fk, fk3, fk4, pk; +-- both sides partitioned +CREATE TABLE pk (a int PRIMARY KEY) PARTITION BY RANGE (a); +CREATE TABLE pk1 PARTITION OF pk FOR VALUES FROM (0) TO (100); +CREATE UNLOGGED TABLE fk (x int REFERENCES pk) PARTITION BY RANGE (x); +CREATE UNLOGGED TABLE fk1 PARTITION OF fk FOR VALUES FROM (0) TO (100); +CREATE UNLOGGED TABLE pk2 PARTITION OF pk FOR VALUES FROM (100) TO (200); +CREATE TABLE fk2 PARTITION OF fk FOR VALUES FROM (100) TO (200); -- fails +ALTER TABLE fk1 SET LOGGED; -- fails +ALTER TABLE pk DETACH PARTITION pk2; +DROP TABLE pk2; +CREATE TABLE fk2 PARTITION OF fk FOR VALUES FROM (100) TO (200); +CREATE UNLOGGED TABLE pk2 PARTITION OF pk FOR VALUES FROM (100) TO (200); -- fails +ALTER TABLE pk1 SET UNLOGGED; -- fails +-- ok, unlogged partitions of the referencing side may still be added +CREATE UNLOGGED TABLE fk3 PARTITION OF fk FOR VALUES FROM (200) TO (300); +DROP TABLE fk, pk; +-- self-referencing +CREATE TABLE self (a int PRIMARY KEY, b int REFERENCES self) PARTITION BY RANGE (a); +CREATE TABLE self1 PARTITION OF self FOR VALUES FROM (0) TO (100); +CREATE UNLOGGED TABLE self2 PARTITION OF self FOR VALUES FROM (100) TO (200); -- fails +CREATE TABLE self2 PARTITION OF self FOR VALUES FROM (100) TO (200); +ALTER TABLE self2 SET UNLOGGED; -- fails +DROP TABLE self; +-- ok, the table being changed is not counted twice +CREATE TABLE self (a int PRIMARY KEY, b int REFERENCES self) PARTITION BY RANGE (a); +CREATE UNLOGGED TABLE self1 PARTITION OF self FOR VALUES FROM (0) TO (100); +ALTER TABLE self1 SET LOGGED; +DROP TABLE self; +DROP SCHEMA fkpart14; +RESET search_path; -- 2.56.0