From 73211a6ddb8ef9b52f846c80ac2eda8bc670d604 Mon Sep 17 00:00:00 2001 From: "Paul A. Jungwirth" Date: Wed, 30 Sep 2026 12:02:19 -0700 Subject: [PATCH v1 1/2] Fix temporal FKs referencing invalid constraints A foreign key with a PERIOD part could reference an exclusion constraint (as long as you said PERIOD in the REFERENCES part of the constraint command). But that is not good enough: it needs to be a WITHOUT OVERLAPS primary key or unique constraint. For one thing, an exclusion constraint doesn't guarantee uniqueness: it doesn't prevent empty ranges, and we don't even check what operators the user chose. For another, UPDATing it doesn't conflict with FOR KEY SHARE, so the foreign key would permit serialization anomalies. This commit forbids that combination. We also detect attempts to reference actual primary keys that lack WITHOUT OVERLAPS. This failed before, but with an unhelpful error message. Existing databases from 18.x may already have such foreign keys. Restoring them would fail, so we added a check to pg_upgrade so that users can replace them with references to WITHOUT OVERLAPS constraints before upgrading. For the release notes: users should look for affected foreign keys with this query, and drop and recreate them referencing a primary key or unique constraint using WITHOUT OVERLAPS: SELECT c.conrelid::regclass AS table_name, c.conname AS constraint_name FROM pg_constraint c JOIN pg_index i ON i.indexrelid = c.conindid WHERE c.contype = 'f' AND c.conperiod AND NOT i.indisunique; Reported-By: Andres Freund Author: Paul A. Jungwirth Discussion: https://www.postgresql.org/message-id/sesqxzfcujkcnrwpir5xvhy7iamhsbjiqt6r2xqmb7hqkulte6%40bbuc42qzanzs Backpatch-through: 18 --- src/backend/commands/tablecmds.c | 21 +++-- src/bin/pg_upgrade/check.c | 88 +++++++++++++++++++ .../regress/expected/without_overlaps.out | 46 ++++++++++ src/test/regress/sql/without_overlaps.sql | 44 ++++++++++ 4 files changed, 193 insertions(+), 6 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 0274d892f2e..859d2b78da7 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -10305,6 +10305,15 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, with_period, opclasses, &pk_has_without_overlaps); } + /* + * If the referencing foreign key uses PERIOD, the primary key must use + * WITHOUT OVERLAPS + */ + if (!pk_has_without_overlaps && with_period) + ereport(ERROR, + errcode(ERRCODE_INVALID_FOREIGN_KEY), + errmsg("foreign key using PERIOD must reference a primary key or unique constraint using WITHOUT OVERLAPS")); + /* * If the referenced primary key has WITHOUT OVERLAPS, the foreign key * must use PERIOD. @@ -14009,12 +14018,11 @@ transformFkeyCheckAttrs(Relation pkrel, indexStruct = (Form_pg_index) GETSTRUCT(indexTuple); /* - * Must have the right number of columns; must be unique (or if - * temporal then exclusion instead) and not a partial index; forget it - * if there are any expressions, too. Invalid indexes are out as well. + * Must have the right number of columns; must be unique and not a + * partial index; forget it if there are any expressions, too. Invalid + * indexes are out as well. */ - if (indexStruct->indnkeyatts == numattrs && - (with_period ? indexStruct->indisexclusion : indexStruct->indisunique) && + if (indexStruct->indnkeyatts == numattrs && indexStruct->indisunique && indexStruct->indisvalid && heap_attisnull(indexTuple, Anum_pg_index_indpred, NULL) && heap_attisnull(indexTuple, Anum_pg_index_indexprs, NULL)) @@ -14077,7 +14085,8 @@ transformFkeyCheckAttrs(Relation pkrel, /* We need to know whether the index has WITHOUT OVERLAPS */ if (found) - *pk_has_without_overlaps = indexStruct->indisexclusion; + *pk_has_without_overlaps = indexStruct->indisunique && + indexStruct->indisexclusion; } ReleaseSysCache(indexTuple); if (found) diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c index 150c4fc34b9..ffed2221dc6 100644 --- a/src/bin/pg_upgrade/check.c +++ b/src/bin/pg_upgrade/check.c @@ -30,6 +30,7 @@ static void check_for_incompatible_polymorphics(ClusterInfo *cluster); static void check_for_tables_with_oids(ClusterInfo *cluster); static void check_for_not_null_inheritance(ClusterInfo *cluster); static void check_for_gist_inet_ops(ClusterInfo *cluster); +static void check_for_temporal_fks_to_exclusion_constraints(ClusterInfo *cluster); static void check_for_new_tablespace_dir(void); static void check_for_user_defined_encoding_conversions(ClusterInfo *cluster); static void check_for_unicode_update(ClusterInfo *cluster); @@ -688,6 +689,15 @@ check_and_dump_old_cluster(void) if (GET_MAJOR_VERSION(old_cluster.major_version) <= 1800) check_for_gist_inet_ops(&old_cluster); + /* + * PG 18 allowed a foreign key with PERIOD to reference a plain exclusion + * constraint, but that is no longer allowed, so restoring such a foreign + * key would fail. Minor releases fixed this, but a cluster could still + * have foreign keys created before the fix. + */ + if (GET_MAJOR_VERSION(old_cluster.major_version) >= 1800) + check_for_temporal_fks_to_exclusion_constraints(&old_cluster); + /* * While not a check option, we do this now because this is the only time * the old server is running. @@ -1812,6 +1822,84 @@ check_for_gist_inet_ops(ClusterInfo *cluster) check_ok(); } +/* + * Callback function for processing results of query for + * check_for_temporal_fks_to_exclusion_constraints()'s UpgradeTask. If the + * query returned any rows (i.e., the check failed), write the details to the + * report file. + */ +static void +process_temporal_fks_to_exclusion_constraints(DbInfo *dbinfo, PGresult *res, + void *arg) +{ + UpgradeTaskReport *report = (UpgradeTaskReport *) arg; + int ntups = PQntuples(res); + int i_nspname = PQfnumber(res, "nspname"); + int i_relname = PQfnumber(res, "relname"); + int i_conname = PQfnumber(res, "conname"); + + if (ntups == 0) + return; + + if (report->file == NULL && + (report->file = fopen_priv(report->path, "w")) == NULL) + pg_fatal("could not open file \"%s\": %m", report->path); + + fprintf(report->file, "In database: %s\n", dbinfo->db_name); + + for (int rowno = 0; rowno < ntups; rowno++) + fprintf(report->file, " %s.%s.%s\n", + PQgetvalue(res, rowno, i_nspname), + PQgetvalue(res, rowno, i_relname), + PQgetvalue(res, rowno, i_conname)); +} + +/* + * Verify that no foreign keys with PERIOD reference a plain exclusion + * constraint instead of a primary key or unique constraint using WITHOUT + * OVERLAPS. Such foreign keys can no longer be created, so they would fail + * to restore. + */ +static void +check_for_temporal_fks_to_exclusion_constraints(ClusterInfo *cluster) +{ + UpgradeTaskReport report; + UpgradeTask *task = upgrade_task_create(); + const char *query = "SELECT n.nspname, c.relname, con.conname " + "FROM pg_catalog.pg_constraint con, pg_catalog.pg_index i, " + " pg_catalog.pg_class c, pg_catalog.pg_namespace n " + "WHERE con.contype = 'f' AND con.conperiod" + " AND con.conindid = i.indexrelid AND NOT i.indisunique" + " AND con.conrelid = c.oid AND c.relnamespace = n.oid"; + + prep_status("Checking for foreign keys referencing exclusion constraints"); + + report.file = NULL; + snprintf(report.path, sizeof(report.path), "%s/%s", + log_opts.basedir, + "temporal_fks_to_exclusion_constraints.txt"); + + upgrade_task_add_step(task, query, + process_temporal_fks_to_exclusion_constraints, + true, &report); + upgrade_task_run(task, cluster); + upgrade_task_free(task); + + if (report.file) + { + fclose(report.file); + pg_log(PG_REPORT, "fatal"); + pg_fatal("Your installation contains foreign keys using PERIOD that reference an\n" + "exclusion constraint instead of a primary key or unique constraint using\n" + "WITHOUT OVERLAPS. These are no longer allowed. Drop them, and recreate\n" + "them referencing a primary key or unique constraint using WITHOUT OVERLAPS.\n" + "A list of foreign keys with the problem is in the file:\n" + " %s", report.path); + } + else + check_ok(); +} + /* * Callback function for processing results of query for * check_for_user_defined_encoding_conversions()'s UpgradeTask. If the query diff --git a/src/test/regress/expected/without_overlaps.out b/src/test/regress/expected/without_overlaps.out index 8e3f6eee0b0..efdf79c17dd 100644 --- a/src/test/regress/expected/without_overlaps.out +++ b/src/test/regress/expected/without_overlaps.out @@ -1573,6 +1573,52 @@ Foreign-key constraints: "temporal_fk2_rng2rng_fk" FOREIGN KEY (parent_id1, parent_id2, PERIOD valid_at) REFERENCES temporal_rng2(id1, id2, PERIOD valid_at) DROP TABLE temporal_fk2_rng2rng; +-- Referencing a non-temporal primary key should fail: +CREATE TABLE temporal_rng3 ( + id int4range, + valid_at daterange, + CONSTRAINT temporal_rng3_pk PRIMARY KEY (id, valid_at) +); +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) +); +ERROR: foreign key using PERIOD must reference a primary key or unique constraint using WITHOUT OVERLAPS +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 +); +ERROR: foreign key using PERIOD must reference a primary key or unique constraint using WITHOUT OVERLAPS +DROP TABLE temporal_rng3; +-- Referencing an exclusion constraint should fail: +CREATE TABLE temporal_rng3 ( + id int4range, + valid_at daterange, + 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) +); +ERROR: there is no unique constraint matching given keys for referenced table "temporal_rng3" +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 +); +ERROR: there is no primary key for referenced table "temporal_rng3" +DROP TABLE temporal_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 46bdd7856a4..786f3066a84 100644 --- a/src/test/regress/sql/without_overlaps.sql +++ b/src/test/regress/sql/without_overlaps.sql @@ -1119,6 +1119,50 @@ CREATE TABLE temporal_fk2_rng2rng ( \d temporal_fk2_rng2rng DROP TABLE temporal_fk2_rng2rng; +-- Referencing a non-temporal primary key should fail: +CREATE TABLE temporal_rng3 ( + id int4range, + valid_at daterange, + CONSTRAINT temporal_rng3_pk PRIMARY KEY (id, valid_at) +); +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) +); +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 +); +DROP TABLE temporal_rng3; + +-- Referencing an exclusion constraint should fail: +CREATE TABLE temporal_rng3 ( + id int4range, + valid_at daterange, + 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) +); +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 +); +DROP TABLE temporal_rng3; + -- -- test ALTER TABLE ADD CONSTRAINT -- -- 2.47.3