From bd21a61b6ee730ee357d9e7292c07b62ea9ab1e0 Mon Sep 17 00:00:00 2001 From: "Paul A. Jungwirth" Date: Wed, 30 Sep 2026 12:02:51 -0700 Subject: [PATCH v1 2/2] Warn about temporal FKs that reference exclusion constraints This is for REL_18_STABLE only. The previous commit stopped foreign keys with PERIOD from referencing a plain exclusion constraint, but earlier 18.x releases allowed it, so databases may already have such foreign keys. They can't reliably enforce referential integrity, and restoring from an old pg_dump will fail. So this commit warns users when we notice them. We warn when building an RI plan for such a foreign key and also when cloning one for a new partition. Later branches don't need this: there these foreign keys could only come from 18.x, and restoring them (with pg_dump or pg_upgrade) fails with the new error. Author: Paul A. Jungwirth Discussion: https://www.postgresql.org/message-id/sesqxzfcujkcnrwpir5xvhy7iamhsbjiqt6r2xqmb7hqkulte6%40bbuc42qzanzs --- src/backend/commands/tablecmds.c | 40 +++++++++++ src/backend/utils/adt/ri_triggers.c | 38 ++++++++++ .../regress/expected/without_overlaps.out | 72 +++++++++++++++++++ src/test/regress/sql/without_overlaps.sql | 61 ++++++++++++++++ 4 files changed, 211 insertions(+) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 859d2b78da7..a8c26de7e5a 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -613,6 +613,8 @@ static void CloneForeignKeyConstraints(List **wqueue, Relation parentRel, static void CloneFkReferenced(Relation parentRel, Relation partitionRel); static void CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel); +static void warnIfPeriodFkIndexNotUnique(bool with_period, Oid indexOid, + const char *conname, Oid relid); static void createForeignKeyCheckTriggers(Oid myRelOid, Oid refRelOid, Constraint *fkconstraint, Oid constraintOid, Oid indexOid, @@ -11501,6 +11503,9 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) conkey, conpfeqop, conppeqop, conffeqop, numfkdelsetcols, confdelsetcols, false, constrForm->conperiod); + warnIfPeriodFkIndexNotUnique(constrForm->conperiod, partIndexId, + fkconstraint->conname, + constrForm->conrelid); /* ... and recurse */ addFkRecurseReferenced(fkconstraint, fkRel, @@ -11746,6 +11751,9 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) conppeqop, conffeqop, numfkdelsetcols, confdelsetcols, false, with_period); + warnIfPeriodFkIndexNotUnique(with_period, indexOid, + get_constraint_name(address.objectId), + RelationGetRelid(partRel)); /* Done with the cloned constraint's tuple */ ReleaseSysCache(tuple); @@ -11776,6 +11784,38 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) table_close(trigrel, RowExclusiveLock); } +/* + * warnIfPeriodFkIndexNotUnique + * + * Earlier 18.x releases let a PERIOD foreign key reference a plain exclusion + * constraint. ATAddForeignKeyConstraint rejects that now, but cloning such a + * foreign key for a new partition copies its index without that check, so + * warn when it happens. + */ +static void +warnIfPeriodFkIndexNotUnique(bool with_period, Oid indexOid, + const char *conname, Oid relid) +{ + HeapTuple indtup; + bool isunique; + + if (!with_period) + return; + + indtup = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(indexOid)); + if (!HeapTupleIsValid(indtup)) + elog(ERROR, "cache lookup failed for index %u", indexOid); + isunique = ((Form_pg_index) GETSTRUCT(indtup))->indisunique; + ReleaseSysCache(indtup); + + if (!isunique) + ereport(WARNING, + errmsg("foreign key constraint \"%s\" on table \"%s\" references an exclusion constraint instead of a primary key or unique constraint using WITHOUT OVERLAPS", + conname, get_rel_name(relid)), + errdetail("Such a foreign key cannot reliably enforce referential integrity."), + errhint("Drop the foreign key and recreate it referencing a primary key or unique constraint using WITHOUT OVERLAPS.")); +} + /* * When the parent of a partition receives [the referencing side of] a foreign * key, we must propagate that foreign key to the partition. However, the diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index 40c1591ac7b..878d185bfc2 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -138,6 +138,7 @@ typedef struct RI_ConstraintInfo * delete */ char confmatchtype; /* foreign key's match type */ bool hasperiod; /* if the foreign key uses PERIOD */ + bool pk_not_unique; /* PERIOD FK references a non-unique EXCLUDE */ int nkeys; /* number of key columns */ int16 pk_attnums[RI_MAX_NUMKEYS]; /* attnums of referenced cols */ int16 fk_attnums[RI_MAX_NUMKEYS]; /* attnums of referencing cols */ @@ -364,6 +365,7 @@ static RI_ConstraintInfo *ri_FetchConstraintInfo(Trigger *trigger, Relation trig_rel, bool rel_is_pk); static RI_ConstraintInfo *ri_LoadConstraintInfo(Oid constraintOid); static Oid get_ri_constraint_root(Oid constrOid); +static void ri_WarnIfNotUnique(const RI_ConstraintInfo *riinfo); static SPIPlanPtr ri_PlanCheck(const char *querystr, int nargs, const Oid *argtypes, RI_QueryKey *qkey, Relation fk_rel, Relation pk_rel); static bool ri_PerformCheck(const RI_ConstraintInfo *riinfo, @@ -648,6 +650,8 @@ RI_FKey_check(TriggerData *trigdata) appendStringInfoString(&querybuf, "(x1.r)"); } + ri_WarnIfNotUnique(riinfo); + /* Prepare and save the plan */ qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys, queryoids, &qkey, fk_rel, pk_rel); @@ -817,6 +821,8 @@ ri_Check_Pk_Match(Relation pk_rel, Relation fk_rel, appendStringInfoString(&querybuf, "(x1.r)"); } + ri_WarnIfNotUnique(riinfo); + /* Prepare and save the plan */ qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys, queryoids, &qkey, fk_rel, pk_rel); @@ -1093,6 +1099,8 @@ ri_restrict(TriggerData *trigdata, bool is_no_action) appendStringInfoString(&querybuf, " FOR KEY SHARE OF x"); + ri_WarnIfNotUnique(riinfo); + /* Prepare and save the plan */ qplan = ri_PlanCheck(querybuf.data, riinfo->nkeys, queryoids, &qkey, fk_rel, pk_rel); @@ -2039,6 +2047,7 @@ RI_PartitionRemove_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel) int i; riinfo = ri_FetchConstraintInfo(trigger, fk_rel, false); + ri_WarnIfNotUnique(riinfo); /* * We don't check permissions before displaying the error message, on the @@ -2546,14 +2555,22 @@ ri_LoadConstraintInfo(Oid constraintOid) * opclass of the PK element for these. This all gets cached (as does the * generated plan), so there's no performance issue. */ + riinfo->pk_not_unique = false; if (riinfo->hasperiod) { Oid opclass = get_index_column_opclass(conForm->conindid, riinfo->nkeys); + HeapTuple indtup; FindFKPeriodOpers(opclass, &riinfo->period_contained_by_oper, &riinfo->agged_period_contained_by_oper, &riinfo->period_intersect_oper); + + indtup = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(conForm->conindid)); + if (!HeapTupleIsValid(indtup)) + elog(ERROR, "cache lookup failed for index %u", conForm->conindid); + riinfo->pk_not_unique = !((Form_pg_index) GETSTRUCT(indtup))->indisunique; + ReleaseSysCache(indtup); } /* Metadata used by fast path. */ @@ -2688,6 +2705,27 @@ InvalidateConstraintCacheCallBack(Datum arg, SysCacheIdentifier cacheid, } +/* + * ri_WarnIfNotUnique - + * + * Earlier 18.x releases let a foreign key with PERIOD reference a plain + * exclusion constraint, which doesn't guarantee uniqueness, so such a foreign + * key can't reliably enforce referential integrity. We no longer allow + * creating one, but some may already exist, so warn when we find one. + */ +static void +ri_WarnIfNotUnique(const RI_ConstraintInfo *riinfo) +{ + if (!riinfo->pk_not_unique) + return; + + ereport(WARNING, + errmsg("foreign key constraint \"%s\" on table \"%s\" references an exclusion constraint instead of a primary key or unique constraint using WITHOUT OVERLAPS", + NameStr(riinfo->conname), get_rel_name(riinfo->fk_relid)), + errdetail("Such a foreign key cannot reliably enforce referential integrity."), + errhint("Drop the foreign key and recreate it referencing a primary key or unique constraint using WITHOUT OVERLAPS.")); +} + /* * Prepare execution plan for a query to enforce an RI restriction */ diff --git a/src/test/regress/expected/without_overlaps.out b/src/test/regress/expected/without_overlaps.out index efdf79c17dd..46e0ff2edcf 100644 --- a/src/test/regress/expected/without_overlaps.out +++ b/src/test/regress/expected/without_overlaps.out @@ -1619,6 +1619,78 @@ CREATE TABLE temporal_fk_rng2rng ( ); ERROR: there is no primary key for referenced table "temporal_rng3" DROP TABLE temporal_rng3; +-- Older releases allowed referencing an exclusion constraint. We can't +-- create such a foreign key any more, so simulate one by pointing a valid +-- foreign key at an exclusion constraint's index. Checking it should warn, +-- but only when we build the plan, not for every row. +CREATE TABLE temporal_rng3 ( + id int4range, + valid_at daterange, + CONSTRAINT temporal_rng3_pk PRIMARY KEY (id, valid_at WITHOUT OVERLAPS), + CONSTRAINT temporal_rng3_excl EXCLUDE USING gist (id WITH =, valid_at WITH &&) +); +CREATE TABLE temporal_fk_rng2rng ( + id int4range, + valid_at daterange, + parent_id int4range, + CONSTRAINT temporal_fk_rng2rng_fk FOREIGN KEY (parent_id, PERIOD valid_at) + REFERENCES temporal_rng3 (id, PERIOD valid_at) +); +UPDATE pg_constraint SET conindid = 'temporal_rng3_excl'::regclass + WHERE conname = 'temporal_fk_rng2rng_fk'; +INSERT INTO temporal_rng3 (id, valid_at) VALUES ('[1,2)', daterange('2018-01-01', '2020-01-01')); +INSERT INTO temporal_fk_rng2rng (id, valid_at, parent_id) VALUES + ('[1,2)', daterange('2018-01-01', '2019-01-01'), '[1,2)'), + ('[2,3)', daterange('2019-01-01', '2020-01-01'), '[1,2)'); +WARNING: foreign key constraint "temporal_fk_rng2rng_fk" on table "temporal_fk_rng2rng" references an exclusion constraint instead of a primary key or unique constraint using WITHOUT OVERLAPS +DETAIL: Such a foreign key cannot reliably enforce referential integrity. +HINT: Drop the foreign key and recreate it referencing a primary key or unique constraint using WITHOUT OVERLAPS. +INSERT INTO temporal_fk_rng2rng (id, valid_at, parent_id) VALUES + ('[3,4)', daterange('2018-01-01', '2019-01-01'), '[1,2)'); +DROP TABLE temporal_fk_rng2rng; +-- Cloning such a foreign key to a new partition should warn too. +CREATE TABLE temporal_partitioned_fk_rng2rng ( + id int4range, + valid_at daterange, + parent_id int4range, + CONSTRAINT temporal_partitioned_fk_rng2rng_fk FOREIGN KEY (parent_id, PERIOD valid_at) + REFERENCES temporal_rng3 (id, PERIOD valid_at) +) PARTITION BY LIST (id); +UPDATE pg_constraint SET conindid = 'temporal_rng3_excl'::regclass + WHERE conname = 'temporal_partitioned_fk_rng2rng_fk'; +CREATE TABLE tfkp1 PARTITION OF temporal_partitioned_fk_rng2rng FOR VALUES IN ('[1,2)'); +WARNING: foreign key constraint "temporal_partitioned_fk_rng2rng_fk" on table "tfkp1" references an exclusion constraint instead of a primary key or unique constraint using WITHOUT OVERLAPS +DETAIL: Such a foreign key cannot reliably enforce referential integrity. +HINT: Drop the foreign key and recreate it referencing a primary key or unique constraint using WITHOUT OVERLAPS. +CREATE TABLE tfkp2 (LIKE temporal_partitioned_fk_rng2rng); +ALTER TABLE temporal_partitioned_fk_rng2rng ATTACH PARTITION tfkp2 FOR VALUES IN ('[2,3)'); +WARNING: foreign key constraint "temporal_partitioned_fk_rng2rng_fk" on table "tfkp2" references an exclusion constraint instead of a primary key or unique constraint using WITHOUT OVERLAPS +DETAIL: Such a foreign key cannot reliably enforce referential integrity. +HINT: Drop the foreign key and recreate it referencing a primary key or unique constraint using WITHOUT OVERLAPS. +DROP TABLE temporal_partitioned_fk_rng2rng; +DROP TABLE temporal_rng3; +-- Likewise when the referenced table is partitioned. +CREATE TABLE temporal_partitioned_rng3 ( + id int4range, + valid_at daterange, + CONSTRAINT temporal_partitioned_rng3_pk PRIMARY KEY (id, valid_at WITHOUT OVERLAPS), + CONSTRAINT temporal_partitioned_rng3_excl EXCLUDE USING gist (id WITH =, valid_at WITH &&) +) PARTITION BY LIST (id); +CREATE TABLE temporal_fk_rng2rng ( + id int4range, + valid_at daterange, + parent_id int4range, + CONSTRAINT temporal_fk_rng2rng_fk FOREIGN KEY (parent_id, PERIOD valid_at) + REFERENCES temporal_partitioned_rng3 (id, PERIOD valid_at) +); +UPDATE pg_constraint SET conindid = 'temporal_partitioned_rng3_excl'::regclass + WHERE conname = 'temporal_fk_rng2rng_fk'; +CREATE TABLE tp1 PARTITION OF temporal_partitioned_rng3 FOR VALUES IN ('[1,2)'); +WARNING: foreign key constraint "temporal_fk_rng2rng_fk" on table "temporal_fk_rng2rng" references an exclusion constraint instead of a primary key or unique constraint using WITHOUT OVERLAPS +DETAIL: Such a foreign key cannot reliably enforce referential integrity. +HINT: Drop the foreign key and recreate it referencing a primary key or unique constraint using WITHOUT OVERLAPS. +DROP TABLE temporal_fk_rng2rng; +DROP TABLE temporal_partitioned_rng3; -- -- test ALTER TABLE ADD CONSTRAINT -- diff --git a/src/test/regress/sql/without_overlaps.sql b/src/test/regress/sql/without_overlaps.sql index 786f3066a84..2c2dccf4626 100644 --- a/src/test/regress/sql/without_overlaps.sql +++ b/src/test/regress/sql/without_overlaps.sql @@ -1163,6 +1163,67 @@ CREATE TABLE temporal_fk_rng2rng ( ); DROP TABLE temporal_rng3; +-- Older releases allowed referencing an exclusion constraint. We can't +-- create such a foreign key any more, so simulate one by pointing a valid +-- foreign key at an exclusion constraint's index. Checking it should warn, +-- but only when we build the plan, not for every row. +CREATE TABLE temporal_rng3 ( + id int4range, + valid_at daterange, + CONSTRAINT temporal_rng3_pk PRIMARY KEY (id, valid_at WITHOUT OVERLAPS), + CONSTRAINT temporal_rng3_excl EXCLUDE USING gist (id WITH =, valid_at WITH &&) +); +CREATE TABLE temporal_fk_rng2rng ( + id int4range, + valid_at daterange, + parent_id int4range, + CONSTRAINT temporal_fk_rng2rng_fk FOREIGN KEY (parent_id, PERIOD valid_at) + REFERENCES temporal_rng3 (id, PERIOD valid_at) +); +UPDATE pg_constraint SET conindid = 'temporal_rng3_excl'::regclass + WHERE conname = 'temporal_fk_rng2rng_fk'; +INSERT INTO temporal_rng3 (id, valid_at) VALUES ('[1,2)', daterange('2018-01-01', '2020-01-01')); +INSERT INTO temporal_fk_rng2rng (id, valid_at, parent_id) VALUES + ('[1,2)', daterange('2018-01-01', '2019-01-01'), '[1,2)'), + ('[2,3)', daterange('2019-01-01', '2020-01-01'), '[1,2)'); +INSERT INTO temporal_fk_rng2rng (id, valid_at, parent_id) VALUES + ('[3,4)', daterange('2018-01-01', '2019-01-01'), '[1,2)'); +DROP TABLE temporal_fk_rng2rng; +-- Cloning such a foreign key to a new partition should warn too. +CREATE TABLE temporal_partitioned_fk_rng2rng ( + id int4range, + valid_at daterange, + parent_id int4range, + CONSTRAINT temporal_partitioned_fk_rng2rng_fk FOREIGN KEY (parent_id, PERIOD valid_at) + REFERENCES temporal_rng3 (id, PERIOD valid_at) +) PARTITION BY LIST (id); +UPDATE pg_constraint SET conindid = 'temporal_rng3_excl'::regclass + WHERE conname = 'temporal_partitioned_fk_rng2rng_fk'; +CREATE TABLE tfkp1 PARTITION OF temporal_partitioned_fk_rng2rng FOR VALUES IN ('[1,2)'); +CREATE TABLE tfkp2 (LIKE temporal_partitioned_fk_rng2rng); +ALTER TABLE temporal_partitioned_fk_rng2rng ATTACH PARTITION tfkp2 FOR VALUES IN ('[2,3)'); +DROP TABLE temporal_partitioned_fk_rng2rng; +DROP TABLE temporal_rng3; +-- Likewise when the referenced table is partitioned. +CREATE TABLE temporal_partitioned_rng3 ( + id int4range, + valid_at daterange, + CONSTRAINT temporal_partitioned_rng3_pk PRIMARY KEY (id, valid_at WITHOUT OVERLAPS), + CONSTRAINT temporal_partitioned_rng3_excl EXCLUDE USING gist (id WITH =, valid_at WITH &&) +) PARTITION BY LIST (id); +CREATE TABLE temporal_fk_rng2rng ( + id int4range, + valid_at daterange, + parent_id int4range, + CONSTRAINT temporal_fk_rng2rng_fk FOREIGN KEY (parent_id, PERIOD valid_at) + REFERENCES temporal_partitioned_rng3 (id, PERIOD valid_at) +); +UPDATE pg_constraint SET conindid = 'temporal_partitioned_rng3_excl'::regclass + WHERE conname = 'temporal_fk_rng2rng_fk'; +CREATE TABLE tp1 PARTITION OF temporal_partitioned_rng3 FOR VALUES IN ('[1,2)'); +DROP TABLE temporal_fk_rng2rng; +DROP TABLE temporal_partitioned_rng3; + -- -- test ALTER TABLE ADD CONSTRAINT -- -- 2.47.3