Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1t1oLG-000WFo-Kd for pgsql-hackers@arkaria.postgresql.org; Fri, 18 Oct 2024 14:52:39 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1t1oLE-005nxl-JQ for pgsql-hackers@arkaria.postgresql.org; Fri, 18 Oct 2024 14:52:36 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1t1oLD-005nxb-DR for pgsql-hackers@lists.postgresql.org; Fri, 18 Oct 2024 14:52:36 +0000 Received: from fhigh-a4-smtp.messagingengine.com ([103.168.172.155]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1t1oL4-001j7y-Nv for pgsql-hackers@postgresql.org; Fri, 18 Oct 2024 14:52:35 +0000 Received: from phl-compute-10.internal (phl-compute-10.phl.internal [10.202.2.50]) by mailfhigh.phl.internal (Postfix) with ESMTP id 9E73B1140169; Fri, 18 Oct 2024 10:52:24 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-10.internal (MEProxy); Fri, 18 Oct 2024 10:52:24 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :reply-to:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1729263144; x= 1729349544; bh=PauDTh3yk6Cg2vJwJ0C3DTKFmVSSwd4nnL3KaHnRYE4=; b=J B2D7XADbTOXqyS3H1uQRfAs6TMASg9MezHSg40iShkRbDwUQAri8uoko3f5e5Pd8 xvU8+WIikTL+vrNFtcEL5CDXmTFrqjLQ4Mnu250OhEdHgo6D0qqJ1nmM+ePBT4ie 1wPbO/J41qaNfrqQHhrHwncbjGVTVFZJS+DXLJbTsxjPsICsVROztarpqn9BbM9B GhdijKCYutoPjON19o2QJ9E1OSrIMy0wOScOxgmqVqT8MRZyNn6r/G+6/SSjR3cU UxaZlsGWbWxPv0c1CQ3HYNMAjeKrOuZrZCAvw4xKnfps+LoUf/yTVAMwoQstR/HP ksBd3CJkzozeci137tySw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrvdehfedgkedtucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepfffhvfevuffkgggtugfgjgesmhekreertddtjeen ucfhrhhomheptehlvhgrrhhoucfjvghrrhgvrhgruceorghlvhhhvghrrhgvsegrlhhvhh drnhhoqdhiphdrohhrgheqnecuggftrfgrthhtvghrnhepudelkeekgfdttdettdekgfdu vdeiffelteegjeeihfetheekveeftedtuddvhfeunecuffhomhgrihhnpegvnhhtvghrph hrihhsvggusgdrtghomhenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgr ihhlfhhrohhmpegrlhhvhhgvrhhrvgesrghlvhhhrdhnohdqihhprdhorhhgpdhnsggprh gtphhtthhopeejpdhmohguvgepshhmthhpohhuthdprhgtphhtthhopehjghgurhesuggr lhhisghordgtohhmpdhrtghpthhtohepvgigtghluhhsihhonhesghhmrghilhdrtghomh dprhgtphhtthhopehtnhgurhifrghnghesghhmrghilhdrtghomhdprhgtphhtthhopeii hhhjfihpkhhusehgmhgrihhlrdgtohhmpdhrtghpthhtohepghhuihhllhgruhhmvgeslh gvlhgrrhhgvgdrihhnfhhopdhrtghpthhtohepmhhitghhrggvlhesphgrqhhuihgvrhdr giihiidprhgtphhtthhopehpghhsqhhlqdhhrggtkhgvrhhssehpohhsthhgrhgvshhqlh drohhrgh X-ME-Proxy: Feedback-ID: ia2694551:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 18 Oct 2024 10:52:10 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alvh.no-ip.org; s=schmee; t=1729263059; bh=UJf1o2KwOnxi/8Qp7hne3VE67IkmVmJs3/gOseqnjeQ=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=EQvgy9Kx8Rx/Y3lM1jh67Ym5pLBA+ib1V2J9g6+jWBib00XC8RRfopxfv3BtgY18I OR4N3yXhc+DFxwfFGLkQxYtPxTw9byMh/bzCdEI9Idp5eXWwimmP9JjvL8DH+H0T/Z gnbieSgu7/iOVj1/j/Jjox9U54VeQqhGb1ZSHGdbXOlPg6PBGpwbgQ7m122o8Tkm8O bFA5lAFLU3ShuJr3K4DIKKmEWliOP6VSSzWarzlYnTegSCj8eKdnkc6VaiI8oxyP6v Ci6pCFbVRtiwxp6VmWmiOO9c4SXVEtKLNmFYKLYaKajMaeKe1ZGP03Jigh8LFRuZnY hWcX2fY5OkT9A== Received: by schmee.alvh.no-ip.org (Postfix, from userid 1000) id C315FA6; Fri, 18 Oct 2024 16:50:59 +0200 (CEST) Date: Fri, 18 Oct 2024 16:50:59 +0200 From: Alvaro Herrera To: Jehan-Guillaume de Rorthais Cc: Tender Wang , Junwang Zhao , "pgsql-hackers@postgresql.org" , Alexander Lakhin , Guillaume Lelarge , Michael Paquier Subject: Re: [BUG] Fix DETACH with FK pointing to a partitioned table fails Message-ID: <202410181450.xchtzcq2uyjt@alvherre.pgsql> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="mr6adwpq5l3ngngx" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20240926104835.07ae9ab5@karst> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --mr6adwpq5l3ngngx Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit On 2024-Sep-26, Jehan-Guillaume de Rorthais wrote: > REL_14_STABLE backport doesn't seem trivial, so I'll wait for some > feedback, review & decision before going further down in backpatching. Hi, thanks for these patches. I have made some edits of my own. In the end I decided that I quite liked the new structure of the addFkConstraint() function and friends. I added a bunch of comments and changed names somewhat. Also, I think the benefit of the refactoring is more obvious when all patches are squashed together, so I did that. For branch 14, I opted to make it delete the constraints on detach. This isn't ideal but unless somebody wants to spend a lot more time on this, it seems the best we can do. Leaving broken constraints around seems worse. The patch for 14 doesn't apply to 13 sadly. I didn't have time to verify why, but it seems there's some rather larger conflict in one spot. I hope to be able to get these pushed over the weekend. That would give us a couple of weeks before the November releases (but my availability in those two weeks might be spotty.) I spent some time going through your test additions and ended up with your functions in this form: -- displays constraints in schema fkpart12 CREATE OR REPLACE FUNCTION fkpart12_constraints(OUT conrel regclass, OUT conname name, OUT confrelid regclass, OUT conparent text) RETURNS SETOF record AS $$ WITH RECURSIVE arrh AS ( SELECT oid, conrelid, conname, confrelid, NULL::name AS conparent FROM pg_constraint WHERE connamespace = 'fkpart12'::regnamespace AND contype = 'f' AND conparentid = 0 UNION ALL SELECT c.oid, c.conrelid, c.conname, c.confrelid, (pg_identify_object('pg_constraint'::regclass, arrh.oid, 0)).identity FROM pg_constraint c JOIN arrh ON c.conparentid = arrh.oid ) SELECT conrelid::regclass, conname, confrelid::regclass, conparent FROM arrh ORDER BY conrelid::regclass::text, conname; $$ LANGUAGE SQL; -- displays triggers in tables in schema fkpart12 CREATE OR REPLACE FUNCTION fkpart12_triggers(OUT tablename regclass, OUT constr text, OUT samefunc boolean, OUT parent text) RETURNS SETOF record AS $$ WITH RECURSIVE arrh AS ( SELECT t.oid, t.tgrelid::regclass as tablename, tgname, t.tgfoid::regproc as trigfn, (pg_identify_object('pg_constraint'::regclass, c.oid, 0)).identity as constr, NULL::bool as samefunc, NULL::name AS parent FROM pg_trigger t LEFT JOIN pg_constraint c ON c.oid = t.tgconstraint WHERE (SELECT relnamespace FROM pg_class WHERE oid = t.tgrelid) = 'fkpart12'::regnamespace AND c.contype = 'f' AND t.tgparentid = 0 UNION ALL SELECT t2.oid, t2.tgrelid::regclass as tablename, t2.tgname, t2.tgfoid::regproc as trigfn, (pg_identify_object('pg_constraint'::regclass, c2.oid, 0)).identity, arrh.trigfn = t2.tgfoid as samefunc, replace((pg_identify_object('pg_trigger'::regclass, t2.tgparentid, 0)).identity, t2.tgparentid::text, 'TGOID') FROM pg_trigger t2 LEFT JOIN pg_constraint c2 ON c2.oid = t2.tgconstraint JOIN arrh ON t2.tgparentid = arrh.oid ) SELECT tablename, constr, samefunc, parent FROM arrh ORDER BY tablename::text, constr; $$ LANGUAGE SQL; However, in the end I think this is a very good technique to verify that the fix works correctly, but it's excessive to include these results in the tests forevermore. So I've left them out for now. Maybe we should reconsider on the older branches? -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "If you have nothing to say, maybe you need just the right tool to help you not say it." (New York Times, about Microsoft PowerPoint) --mr6adwpq5l3ngngx Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v4_14-0001-Redo-foreign-key-maintenance-during-partition-.patch" From 87cfd9e5e6942e4287f4a578a8e6d86a66b099a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= Date: Fri, 18 Oct 2024 16:40:34 +0200 Subject: [PATCH v4_14] Redo foreign key maintenance during partition ATTACH/DETACH ... when the reference table is partitioned. It turns out that the catalog representation we chose for foreign keys connecting partitioned tables is suboptimal, particularly in the sense that a standalone table has a different way to represent it when referencing a partitioned table, than when the same table becomes a partition (and vice versa). This difference means we need to create additional catalog rows on detach (pg_constraint and pg_trigger), and remove them on attach. As a very obvious symptom, we were missing action triggers after detach, which means that you could update/delete rows from the referenced partitioned table that still had referencing rows at the other side previously detached, and fail to throw the required errors. !!!!!!!!! Note that this means existing FKs that refer to partitioned tables might have rows that break relational integrity, on tables that were once partitions. Another possible problem is that trying to reattach a table that had been detached would fail indicating that internal triggers cannot be found, which from the user's point of view is nonsensical. In branches 15 and above, we fix this by creating a new helper function addFkConstraint() which is in charge of creating a standalone pg_constraint row, and repurposing addFkRecurseReferencing() and addFkRecurseReferenced() so that they're only the recursive routine for each side of the FK, and they call addFkConstraint() to create pg_constraint at each partitioning level and add the necessary triggers. These new routines can be used during partition creation, partition attach and detach, and foreign key creation. This reduces redundant code and simplifies the flow. In branches 14 and 13, we have a much simpler fix that consists on simply removing the constraint on detach. The reason is that those branches are missing commit f4566345cf40, which reworked the way this works in a way that we didn't consider back-patchable at the time. I doubt we want to change that now, so this is probably the best we can do there. We opted to leave branch 12 alone, because it's going to end maintenance very soon, and any rushed patch might be worse. In the future we might want to rethink the catalog representation to avoid this problem, but for now the code seems to do what's required to make the constraints operate correctly. Co-authored-by: Jehan-Guillaume de Rorthais Co-authored-by: Tender Wang Co-authored-by: Alvaro Herrera Reported-by: Guillaume Lelarge Reported-by: Jehan-Guillaume de Rorthais Reported-by: Thomas Baehler (SBB CFF FFS) Discussion: https://postgr.es/m/20230420144344.40744130@karst Discussion: https://postgr.es/m/20230705233028.2f554f73@karst Discussion: https://postgr.es/m/GVAP278MB02787E7134FD691861635A8BC9032@GVAP278MB0278.CHEP278.PROD.OUTLOOK.COM Discussion: https://postgr.es/m/18541-628a61bc267cd2d3@postgresql.org --- src/backend/commands/tablecmds.c | 160 ++++++++++++++++++---- src/test/regress/expected/foreign_key.out | 91 ++++++++++++ src/test/regress/sql/foreign_key.sql | 56 ++++++++ 3 files changed, 281 insertions(+), 26 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 38cc7f6b854..6dd4e88d48e 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -10389,6 +10389,82 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk, table_close(trigrel, RowExclusiveLock); ConstraintSetParentConstraint(fk->conoid, parentConstrOid, partRelid); + + /* + * If the referenced table is partitioned, then the partition we're + * attaching now has extra pg_constraint rows and action triggers that are + * no longer needed. Remove those. + */ + if (get_rel_relkind(fk->confrelid) == RELKIND_PARTITIONED_TABLE) + { + Relation pg_constraint = table_open(ConstraintRelationId, RowShareLock); + ObjectAddresses *objs; + HeapTuple consttup; + + ScanKeyInit(&key, + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(fk->conrelid)); + + scan = systable_beginscan(pg_constraint, + ConstraintRelidTypidNameIndexId, + true, NULL, 1, &key); + objs = new_object_addresses(); + while ((consttup = systable_getnext(scan)) != NULL) + { + Form_pg_constraint conform = (Form_pg_constraint) GETSTRUCT(consttup); + + if (conform->conparentid != fk->conoid) + continue; + else + { + ObjectAddress addr; + SysScanDesc scan2; + ScanKeyData key2; + int n PG_USED_FOR_ASSERTS_ONLY; + + ObjectAddressSet(addr, ConstraintRelationId, conform->oid); + add_exact_object_address(&addr, objs); + + /* + * First we must delete the dependency records that bind + * the constraint records together. + */ + n = deleteDependencyRecordsForSpecific(ConstraintRelationId, + conform->oid, + DEPENDENCY_INTERNAL, + ConstraintRelationId, + fk->conoid); + Assert(n == 1); /* actually only one is expected */ + + /* + * Now search for the triggers for this constraint and set + * them up for deletion too + */ + ScanKeyInit(&key2, + Anum_pg_trigger_tgconstraint, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(conform->oid)); + scan2 = systable_beginscan(trigrel, TriggerConstraintIndexId, + true, NULL, 1, &key2); + while ((trigtup = systable_getnext(scan2)) != NULL) + { + ObjectAddressSet(addr, TriggerRelationId, + ((Form_pg_trigger) GETSTRUCT(trigtup))->oid); + add_exact_object_address(&addr, objs); + } + systable_endscan(scan2); + } + } + /* make the dependency deletions visible */ + CommandCounterIncrement(); + performMultipleDeletions(objs, DROP_RESTRICT, + PERFORM_DELETION_INTERNAL); + systable_endscan(scan); + + table_close(pg_constraint, RowShareLock); + } + CommandCounterIncrement(); return true; } @@ -18154,6 +18230,7 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, new_repl[Natts_pg_class]; HeapTuple tuple, newtuple; + ObjectAddresses *dropobjs = NULL; if (concurrent) { @@ -18168,8 +18245,8 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, DropClonedTriggersFromPartition(RelationGetRelid(partRel)); /* - * Detach any foreign keys that are inherited. This includes creating - * additional action triggers. + * Detach any foreign keys that are inherited -- or, if they reference + * partitioned tables, drop them. */ fks = copyObject(RelationGetFKeyList(partRel)); foreach(cell, fks) @@ -18177,7 +18254,6 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, ForeignKeyCacheInfo *fk = lfirst(cell); HeapTuple contup; Form_pg_constraint conform; - Constraint *fkconstraint; contup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(fk->conoid)); if (!HeapTupleIsValid(contup)) @@ -18192,39 +18268,71 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, continue; } - /* unset conparentid and adjust conislocal, coninhcount, etc. */ + /* Mark the constraint as independent */ ConstraintSetParentConstraint(fk->conoid, InvalidOid, InvalidOid); /* - * Make the action triggers on the referenced relation. When this was - * a partition the action triggers pointed to the parent rel (they - * still do), but now we need separate ones of our own. + * If the constraint references a partitioned table, just drop the + * constraint, because it's more work to preserve the constraint + * correctly. + * + * If it references a plain table, then we can create the action + * triggers and it'll be okay. */ - fkconstraint = makeNode(Constraint); - fkconstraint->contype = CONSTRAINT_FOREIGN; - fkconstraint->conname = pstrdup(NameStr(conform->conname)); - fkconstraint->deferrable = conform->condeferrable; - fkconstraint->initdeferred = conform->condeferred; - fkconstraint->location = -1; - fkconstraint->pktable = NULL; - fkconstraint->fk_attrs = NIL; - fkconstraint->pk_attrs = NIL; - fkconstraint->fk_matchtype = conform->confmatchtype; - fkconstraint->fk_upd_action = conform->confupdtype; - fkconstraint->fk_del_action = conform->confdeltype; - fkconstraint->old_conpfeqop = NIL; - fkconstraint->old_pktable_oid = InvalidOid; - fkconstraint->skip_validation = false; - fkconstraint->initially_valid = true; + if (get_rel_relkind(fk->confrelid) == RELKIND_PARTITIONED_TABLE) + { + ObjectAddress constraddr; - createForeignKeyActionTriggers(partRel, conform->confrelid, - fkconstraint, fk->conoid, - conform->conindid); + /* make the dependency deletions above visible */ + CommandCounterIncrement(); + + /* + * Remember the constraint and its triggers for later deletion. + */ + if (dropobjs == NULL) + dropobjs = new_object_addresses(); + ObjectAddressSet(constraddr, ConstraintRelationId, fk->conoid); + add_exact_object_address(&constraddr, dropobjs); + } + else + { + Constraint *fkconstraint; + + /* + * Make the action triggers on the referenced relation. When this + * was a partition the action triggers pointed to the parent rel + * (they still do), but now we need separate ones of our own. + */ + fkconstraint = makeNode(Constraint); + fkconstraint->contype = CONSTRAINT_FOREIGN; + fkconstraint->conname = pstrdup(NameStr(conform->conname)); + fkconstraint->deferrable = conform->condeferrable; + fkconstraint->initdeferred = conform->condeferred; + fkconstraint->location = -1; + fkconstraint->pktable = NULL; + fkconstraint->fk_attrs = NIL; + fkconstraint->pk_attrs = NIL; + fkconstraint->fk_matchtype = conform->confmatchtype; + fkconstraint->fk_upd_action = conform->confupdtype; + fkconstraint->fk_del_action = conform->confdeltype; + fkconstraint->old_conpfeqop = NIL; + fkconstraint->old_pktable_oid = InvalidOid; + fkconstraint->skip_validation = false; + fkconstraint->initially_valid = true; + + createForeignKeyActionTriggers(partRel, conform->confrelid, + fkconstraint, fk->conoid, + conform->conindid); + } ReleaseSysCache(contup); } list_free_deep(fks); + /* If we collected any constraints for deletion, do so now. */ + if (dropobjs != NULL) + performMultipleDeletions(dropobjs, DROP_CASCADE, 0); + /* * Any sub-constraints that are in the referenced-side of a larger * constraint have to be removed. This partition is no longer part of the diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index a95fe17d1a7..e17b95021cf 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -2665,3 +2665,94 @@ DROP SCHEMA fkpart10 CASCADE; NOTICE: drop cascades to 2 other objects DETAIL: drop cascades to table fkpart10.tbl1 drop cascades to table fkpart10.tbl2 +-- When a table is attached as partition to a partitioned table that has +-- a foreign key to another partitioned table, it acquires a clone of the +-- FK. Upon detach, Postgres 14 and earlier remove the foreign key (newer +-- versions make it a standalone constraint.) +CREATE SCHEMA fkpart12 + CREATE TABLE fk_p ( id int, jd int, PRIMARY KEY(id, jd)) PARTITION BY list (id) + CREATE TABLE fk_p_1 PARTITION OF fk_p FOR VALUES IN (1) PARTITION BY list (jd) + CREATE TABLE fk_p_1_1 PARTITION OF fk_p_1 FOR VALUES IN (1) + CREATE TABLE fk_p_1_2 PARTITION OF fk_p_1 FOR VALUES IN (2) + CREATE TABLE fk_p_2 PARTITION OF fk_p FOR VALUES IN (2) PARTITION BY list (jd) + CREATE TABLE fk_p_2_1 PARTITION OF fk_p_2 FOR VALUES IN (1) + CREATE TABLE fk_p_2_2 PARTITION OF fk_p_2 FOR VALUES IN (2) + CREATE TABLE fk_r_1 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) + CREATE TABLE fk_r_2 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) PARTITION BY list (id) + CREATE TABLE fk_r_2_1 PARTITION OF fk_r_2 FOR VALUES IN (2, 1) + CREATE TABLE fk_r ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL, + FOREIGN KEY (p_id, p_jd) REFERENCES fk_p (id, jd) + ) PARTITION BY list (id); +SET search_path TO fkpart12; +INSERT INTO fk_p VALUES (1, 1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2); +\d fk_r_2 + Partitioned table "fkpart12.fk_r_2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + id | integer | | not null | + p_id | integer | | not null | + p_jd | integer | | not null | +Partition of: fk_r FOR VALUES IN (2) +Partition key: LIST (id) +Indexes: + "fk_r_2_pkey" PRIMARY KEY, btree (id) +Foreign-key constraints: + TABLE "fk_r" CONSTRAINT "fk_r_p_id_p_jd_fkey" FOREIGN KEY (p_id, p_jd) REFERENCES fk_p(id, jd) +Number of partitions: 1 (Use \d+ to list them.) + +INSERT INTO fk_r VALUES (1, 1, 1); +INSERT INTO fk_r VALUES (2, 2, 1); -- fails +ERROR: insert or update on table "fk_r_2_1" violates foreign key constraint "fk_r_p_id_p_jd_fkey" +DETAIL: Key (p_id, p_jd)=(2, 1) is not present in table "fk_p". +ALTER TABLE fk_r DETACH PARTITION fk_r_1; +ALTER TABLE fk_r DETACH PARTITION fk_r_2; +\d fk_r_2 + Partitioned table "fkpart12.fk_r_2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + id | integer | | not null | + p_id | integer | | not null | + p_jd | integer | | not null | +Partition key: LIST (id) +Indexes: + "fk_r_2_pkey" PRIMARY KEY, btree (id) +Number of partitions: 1 (Use \d+ to list them.) + +INSERT INTO fk_r_1 VALUES (2, 1, 2); -- works: there's no FK anymore +DELETE FROM fk_p; -- works +ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); -- fails +ERROR: partition constraint of relation "fk_r_1" is violated by some row +INSERT INTO fk_r_2 VALUES (2, 2, 2); +INSERT INTO fk_p VALUES (2, 2); +ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2); +\d fk_r_2 + Partitioned table "fkpart12.fk_r_2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + id | integer | | not null | + p_id | integer | | not null | + p_jd | integer | | not null | +Partition of: fk_r FOR VALUES IN (2) +Partition key: LIST (id) +Indexes: + "fk_r_2_pkey" PRIMARY KEY, btree (id) +Foreign-key constraints: + TABLE "fk_r" CONSTRAINT "fk_r_p_id_p_jd_fkey" FOREIGN KEY (p_id, p_jd) REFERENCES fk_p(id, jd) +Number of partitions: 1 (Use \d+ to list them.) + +DELETE FROM fk_p; -- fails +ERROR: update or delete on table "fk_p_2_2" violates foreign key constraint "fk_r_p_id_p_jd_fkey6" on table "fk_r" +DETAIL: Key (id, jd)=(2, 2) is still referenced from table "fk_r". +-- these should all fail +ALTER TABLE fk_r_1 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; +ERROR: constraint "fk_r_p_id_p_jd_fkey" of relation "fk_r_1" does not exist +ALTER TABLE fk_r DROP CONSTRAINT fk_r_p_id_p_jd_fkey1; +ERROR: cannot drop inherited constraint "fk_r_p_id_p_jd_fkey1" of relation "fk_r" +ALTER TABLE fk_r_2 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; +ERROR: cannot drop inherited constraint "fk_r_p_id_p_jd_fkey" of relation "fk_r_2" +SET client_min_messages TO warning; +DROP SCHEMA fkpart12 CASCADE; +RESET client_min_messages; +RESET search_path; diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index 2ae74ebb148..7af7d562c7b 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -1902,3 +1902,59 @@ UPDATE fkpart10.tbl1 SET f1 = 2 WHERE f1 = 1; INSERT INTO fkpart10.tbl1 VALUES (0), (1); COMMIT; DROP SCHEMA fkpart10 CASCADE; + +-- When a table is attached as partition to a partitioned table that has +-- a foreign key to another partitioned table, it acquires a clone of the +-- FK. Upon detach, Postgres 14 and earlier remove the foreign key (newer +-- versions make it a standalone constraint.) +CREATE SCHEMA fkpart12 + CREATE TABLE fk_p ( id int, jd int, PRIMARY KEY(id, jd)) PARTITION BY list (id) + CREATE TABLE fk_p_1 PARTITION OF fk_p FOR VALUES IN (1) PARTITION BY list (jd) + CREATE TABLE fk_p_1_1 PARTITION OF fk_p_1 FOR VALUES IN (1) + CREATE TABLE fk_p_1_2 PARTITION OF fk_p_1 FOR VALUES IN (2) + CREATE TABLE fk_p_2 PARTITION OF fk_p FOR VALUES IN (2) PARTITION BY list (jd) + CREATE TABLE fk_p_2_1 PARTITION OF fk_p_2 FOR VALUES IN (1) + CREATE TABLE fk_p_2_2 PARTITION OF fk_p_2 FOR VALUES IN (2) + CREATE TABLE fk_r_1 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) + CREATE TABLE fk_r_2 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) PARTITION BY list (id) + CREATE TABLE fk_r_2_1 PARTITION OF fk_r_2 FOR VALUES IN (2, 1) + CREATE TABLE fk_r ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL, + FOREIGN KEY (p_id, p_jd) REFERENCES fk_p (id, jd) + ) PARTITION BY list (id); +SET search_path TO fkpart12; + +INSERT INTO fk_p VALUES (1, 1); + +ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2); + +\d fk_r_2 + +INSERT INTO fk_r VALUES (1, 1, 1); +INSERT INTO fk_r VALUES (2, 2, 1); -- fails + +ALTER TABLE fk_r DETACH PARTITION fk_r_1; +ALTER TABLE fk_r DETACH PARTITION fk_r_2; + +\d fk_r_2 + +INSERT INTO fk_r_1 VALUES (2, 1, 2); -- works: there's no FK anymore +DELETE FROM fk_p; -- works + +ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); -- fails + +INSERT INTO fk_r_2 VALUES (2, 2, 2); +INSERT INTO fk_p VALUES (2, 2); +ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2); +\d fk_r_2 +DELETE FROM fk_p; -- fails + +-- these should all fail +ALTER TABLE fk_r_1 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; +ALTER TABLE fk_r DROP CONSTRAINT fk_r_p_id_p_jd_fkey1; +ALTER TABLE fk_r_2 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; + +SET client_min_messages TO warning; +DROP SCHEMA fkpart12 CASCADE; +RESET client_min_messages; +RESET search_path; -- 2.39.5 --mr6adwpq5l3ngngx Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v4_15-0001-Redo-foreign-key-maintenance-during-partition-.patch" From d8dd2463a06370d435e63f7448cdf40bab587234 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= Date: Fri, 18 Oct 2024 16:40:35 +0200 Subject: [PATCH v4_15] Redo foreign key maintenance during partition ATTACH/DETACH ... when the reference table is partitioned. It turns out that the catalog representation we chose for foreign keys connecting partitioned tables is suboptimal, particularly in the sense that a standalone table has a different way to represent it when referencing a partitioned table, than when the same table becomes a partition (and vice versa). This difference means we need to create additional catalog rows on detach (pg_constraint and pg_trigger), and remove them on attach. As a very obvious symptom, we were missing action triggers after detach, which means that you could update/delete rows from the referenced partitioned table that still had referencing rows at the other side previously detached, and fail to throw the required errors. !!!!!!!!! Note that this means existing FKs that refer to partitioned tables might have rows that break relational integrity, on tables that were once partitions. Another possible problem is that trying to reattach a table that had been detached would fail indicating that internal triggers cannot be found, which from the user's point of view is nonsensical. In branches 15 and above, we fix this by creating a new helper function addFkConstraint() which is in charge of creating a standalone pg_constraint row, and repurposing addFkRecurseReferencing() and addFkRecurseReferenced() so that they're only the recursive routine for each side of the FK, and they call addFkConstraint() to create pg_constraint at each partitioning level and add the necessary triggers. These new routines can be used during partition creation, partition attach and detach, and foreign key creation. This reduces redundant code and simplifies the flow. In branches 14 and 13, we have a much simpler fix that consists on simply removing the constraint on detach. The reason is that those branches are missing commit f4566345cf40, which reworked the way this works in a way that we didn't consider back-patchable at the time. I doubt we want to change that now, so this is probably the best we can do there. We opted to leave branch 12 alone, because it's going to end maintenance very soon, and any rushed patch might be worse. In the future we might want to rethink the catalog representation to avoid this problem, but for now the code seems to do what's required to make the constraints operate correctly. Co-authored-by: Jehan-Guillaume de Rorthais Co-authored-by: Tender Wang Co-authored-by: Alvaro Herrera Reported-by: Guillaume Lelarge Reported-by: Jehan-Guillaume de Rorthais Reported-by: Thomas Baehler (SBB CFF FFS) Discussion: https://postgr.es/m/20230420144344.40744130@karst Discussion: https://postgr.es/m/20230705233028.2f554f73@karst Discussion: https://postgr.es/m/GVAP278MB02787E7134FD691861635A8BC9032@GVAP278MB0278.CHEP278.PROD.OUTLOOK.COM Discussion: https://postgr.es/m/18541-628a61bc267cd2d3@postgresql.org --- src/backend/commands/tablecmds.c | 631 ++++++++++++++-------- src/test/regress/expected/foreign_key.out | 96 ++++ src/test/regress/sql/foreign_key.sql | 57 ++ src/tools/pgindent/typedefs.list | 1 + 4 files changed, 549 insertions(+), 236 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index e6c911fd9a4..18690dc4af0 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -335,6 +335,14 @@ typedef struct ForeignTruncateInfo List *rels; } ForeignTruncateInfo; +/* Partial or complete FK creation in addFkConstraint() */ +typedef enum addFkConstraintSides +{ + addFkReferencedSide, + addFkReferencingSide, + addFkBothSides, +} addFkConstraintSides; + /* * Partition tables are expected to be dropped when the parent partitioned * table gets dropped. Hence for partitioning we use AUTO dependency. @@ -489,16 +497,25 @@ static ObjectAddress ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo * Relation rel, Constraint *fkconstraint, bool recurse, bool recursing, LOCKMODE lockmode); -static ObjectAddress addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, - Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr, - int numfks, int16 *pkattnum, int16 *fkattnum, - Oid *pfeqoperators, Oid *ppeqoperators, Oid *ffeqoperators, - int numfkdelsetcols, int16 *fkdelsetcols, - bool old_check_ok, - Oid parentDelTrigger, Oid parentUpdTrigger); static void validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums, int numfksetcols, const int16 *fksetcolsattnums, List *fksetcols); +static ObjectAddress addFkConstraint(addFkConstraintSides fkside, + char *constraintname, + Constraint *fkconstraint, Relation rel, + Relation pkrel, Oid indexOid, + Oid parentConstr, + int numfks, int16 *pkattnum, int16 *fkattnum, + Oid *pfeqoperators, Oid *ppeqoperators, + Oid *ffeqoperators, int numfkdelsetcols, + int16 *fkdelsetcols, bool is_internal); +static void addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, + Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr, + int numfks, int16 *pkattnum, int16 *fkattnum, + Oid *pfeqoperators, Oid *ppeqoperators, Oid *ffeqoperators, + int numfkdelsetcols, int16 *fkdelsetcols, + bool old_check_ok, + Oid parentDelTrigger, Oid parentUpdTrigger); static void addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr, int numfks, int16 *pkattnum, int16 *fkattnum, @@ -625,10 +642,12 @@ static void DropClonedTriggersFromPartition(Oid partitionId); static ObjectAddress ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, RangeVar *name, bool concurrent); -static void DetachPartitionFinalize(Relation rel, Relation partRel, - bool concurrent, Oid defaultPartOid); -static ObjectAddress ATExecDetachPartitionFinalize(Relation rel, RangeVar *name); -static ObjectAddress ATExecAttachPartitionIdx(List **wqueue, Relation rel, +static void DetachPartitionFinalize(List **wqueue, Relation rel, + Relation partRel, bool concurrent, + Oid defaultPartOid); +static ObjectAddress ATExecDetachPartitionFinalize(List **wqueue, Relation rel, + RangeVar *name); +static ObjectAddress ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name); static void validatePartitionedIndex(Relation partedIdx, Relation partedTbl); static void refuseDupeIndexAttach(Relation parentIdx, Relation partIdx, @@ -5283,7 +5302,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, ((PartitionCmd *) cmd->def)->concurrent); break; case AT_DetachPartitionFinalize: - ATExecDetachPartitionFinalize(rel, ((PartitionCmd *) cmd->def)->name); + address = ATExecDetachPartitionFinalize(wqueue, rel, ((PartitionCmd *) cmd->def)->name); break; default: /* oops */ elog(ERROR, "unrecognized alter table type: %d", @@ -9524,25 +9543,37 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, ffeqoperators[i] = ffeqop; } - /* - * Create all the constraint and trigger objects, recursing to partitions - * as necessary. First handle the referenced side. - */ - address = addFkRecurseReferenced(wqueue, fkconstraint, rel, pkrel, - indexOid, - InvalidOid, /* no parent constraint */ - numfks, - pkattnum, - fkattnum, - pfeqoperators, - ppeqoperators, - ffeqoperators, - numfkdelsetcols, - fkdelsetcols, - old_check_ok, - InvalidOid, InvalidOid); + /* First, create the constraint catalog entry itself. */ + address = addFkConstraint(addFkBothSides, + fkconstraint->conname, fkconstraint, rel, pkrel, + indexOid, + InvalidOid, /* no parent constraint */ + numfks, + pkattnum, + fkattnum, + pfeqoperators, + ppeqoperators, + ffeqoperators, + numfkdelsetcols, + fkdelsetcols, + false); - /* Now handle the referencing side. */ + /* Next process the action triggers at the referenced side and recurse */ + addFkRecurseReferenced(wqueue, fkconstraint, rel, pkrel, + indexOid, + address.objectId, + numfks, + pkattnum, + fkattnum, + pfeqoperators, + ppeqoperators, + ffeqoperators, + numfkdelsetcols, + fkdelsetcols, + old_check_ok, + InvalidOid, InvalidOid); + + /* Lastly create the check triggers at the referencing side and recurse */ addFkRecurseReferencing(wqueue, fkconstraint, rel, pkrel, indexOid, address.objectId, @@ -9602,46 +9633,43 @@ validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums, } /* - * addFkRecurseReferenced - * subroutine for ATAddForeignKeyConstraint; recurses on the referenced - * side of the constraint + * addFkConstraint + * Install pg_constraint entries to implement a foreign key constraint. + * Caller must separately invoke addFkRecurseReferenced and + * addFkRecurseReferencing, as appropriate, to install pg_trigger entries + * and (for partitioned tables) recurse to partitions. * - * Create pg_constraint rows for the referenced side of the constraint, - * referencing the parent of the referencing side; also create action triggers - * on leaf partitions. If the table is partitioned, recurse to handle each - * partition. - * - * wqueue is the ALTER TABLE work queue; can be NULL when not running as part - * of an ALTER TABLE sequence. - * fkconstraint is the constraint being added. - * rel is the root referencing relation. - * pkrel is the referenced relation; might be a partition, if recursing. - * indexOid is the OID of the index (on pkrel) implementing this constraint. - * parentConstr is the OID of a parent constraint; InvalidOid if this is a - * top-level constraint. - * numfks is the number of columns in the foreign key - * pkattnum is the attnum array of referenced attributes. - * fkattnum is the attnum array of referencing attributes. - * numfkdelsetcols is the number of columns in the ON DELETE SET NULL/DEFAULT - * (...) clause - * fkdelsetcols is the attnum array of the columns in the ON DELETE SET - * NULL/DEFAULT clause - * pf/pp/ffeqoperators are OID array of operators between columns. - * old_check_ok signals that this constraint replaces an existing one that - * was already validated (thus this one doesn't need validation). - * parentDelTrigger and parentUpdTrigger, when being recursively called on - * a partition, are the OIDs of the parent action triggers for DELETE and - * UPDATE respectively. + * * fkside indicates which side of the FK (or both) to create. Caller must + * call addFkRecurseReferenced if it's addFkReferencedSide, + * addFkRecurseReferencing if it's addFkReferencingSide, or both if it's + * addFkBothSides. + * * constraintname the base name for the constraint being added. This is + * copied to fkconstraint->conname if the latter is not set. + * * fkconstraint is the constraint being added. + * * rel is the root referencing relation. + * * pkrel is the referenced relation; might be a partition, if recursing. + * * indexOid is the OID of the index (on pkrel) implementing this constraint. + * * parentConstr is the OID of a parent constraint; InvalidOid if this is a + * top-level constraint. + * * numfks is the number of columns in the foreign key. + * * pkattnum is the attnum array of referenced attributes. + * * fkattnum is the attnum array of referencing attributes. + * * pf/pp/ffeqoperators are OID array of operators between columns. + * * numfkdelsetcols is the number of columns in the ON DELETE SET NULL/DEFAULT + * (...) clause. + * * fkdelsetcols is the attnum array of the columns in the ON DELETE SET + * NULL/DEFAULT clause. + * * is_internal: whether to mark the constraint as internal (used during + * recursion) */ static ObjectAddress -addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, - Relation pkrel, Oid indexOid, Oid parentConstr, - int numfks, - int16 *pkattnum, int16 *fkattnum, Oid *pfeqoperators, - Oid *ppeqoperators, Oid *ffeqoperators, - int numfkdelsetcols, int16 *fkdelsetcols, - bool old_check_ok, - Oid parentDelTrigger, Oid parentUpdTrigger) +addFkConstraint(addFkConstraintSides fkside, + char *constraintname, Constraint *fkconstraint, + Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr, + int numfks, int16 *pkattnum, + int16 *fkattnum, Oid *pfeqoperators, Oid *ppeqoperators, + Oid *ffeqoperators, int numfkdelsetcols, int16 *fkdelsetcols, + bool is_internal) { ObjectAddress address; Oid constrOid; @@ -9649,8 +9677,6 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, bool conislocal; int coninhcount; bool connoinherit; - Oid deleteTriggerOid, - updateTriggerOid; /* * Verify relkind for each referenced partition. At the top level, this @@ -9669,13 +9695,16 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, */ if (ConstraintNameIsUsed(CONSTRAINT_RELATION, RelationGetRelid(rel), - fkconstraint->conname)) + constraintname)) conname = ChooseConstraintName(RelationGetRelationName(rel), ChooseForeignKeyConstraintNameAddition(fkconstraint->fk_attrs), "fkey", RelationGetNamespace(rel), NIL); else - conname = fkconstraint->conname; + conname = constraintname; + + if (fkconstraint->conname == NULL) + fkconstraint->conname = pstrdup(conname); if (OidIsValid(parentConstr)) { @@ -9727,33 +9756,95 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, conislocal, /* islocal */ coninhcount, /* inhcount */ connoinherit, /* conNoInherit */ - false); /* is_internal */ + is_internal); /* is_internal */ ObjectAddressSet(address, ConstraintRelationId, constrOid); - /* - * Mark the child constraint as part of the parent constraint; it must not - * be dropped on its own. (This constraint is deleted when the partition - * is detached, but a special check needs to occur that the partition - * contains no referenced values.) - */ if (OidIsValid(parentConstr)) { ObjectAddress referenced; ObjectAddressSet(referenced, ConstraintRelationId, parentConstr); - recordDependencyOn(&address, &referenced, DEPENDENCY_INTERNAL); + if (fkside == addFkReferencedSide) + { + /* + * Mark the child constraint as part of the parent constraint; it + * must not be dropped on its own. (This constraint is deleted + * when the partition is detached, but a special check needs to + * occur that the partition contains no referenced values.) + */ + recordDependencyOn(&address, &referenced, DEPENDENCY_INTERNAL); + } + else + { + /* + * Give this constraint partition-type dependencies on the parent + * constraint as well as the table. + */ + recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_PRI); + ObjectAddressSet(referenced, RelationRelationId, RelationGetRelid(rel)); + recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_SEC); + } } /* make new constraint visible, in case we add more */ CommandCounterIncrement(); + return address; +} + +/* + * addFkRecurseReferenced + * Recursive helper for the referenced side of foreign key creation, + * which creates the action triggers and recurses + * + * If the referenced relation is a plain relation, create the necessary action + * triggers that implement the constraint. If the referenced relation is a + * partitioned table, then we create a pg_constraint row referencing the parent + * of the referencing side for it and recurse on this routine for each + * partition. + * + * wqueue is the ALTER TABLE work queue; can be NULL when not running as part + * of an ALTER TABLE sequence. + * fkconstraint is the constraint being added. + * rel is the root referencing relation. + * pkrel is the referenced relation; might be a partition, if recursing. + * indexOid is the OID of the index (on pkrel) implementing this constraint. + * parentConstr is the OID of a parent constraint; InvalidOid if this is a + * top-level constraint. + * numfks is the number of columns in the foreign key + * pkattnum is the attnum array of referenced attributes. + * fkattnum is the attnum array of referencing attributes. + * numfkdelsetcols is the number of columns in the ON DELETE SET NULL/DEFAULT + * (...) clause + * fkdelsetcols is the attnum array of the columns in the ON DELETE SET + * NULL/DEFAULT clause + * pf/pp/ffeqoperators are OID array of operators between columns. + * old_check_ok signals that this constraint replaces an existing one that + * was already validated (thus this one doesn't need validation). + * parentDelTrigger and parentUpdTrigger, when being recursively called on + * a partition, are the OIDs of the parent action triggers for DELETE and + * UPDATE respectively. + */ +static void +addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, + Relation pkrel, Oid indexOid, Oid parentConstr, + int numfks, + int16 *pkattnum, int16 *fkattnum, Oid *pfeqoperators, + Oid *ppeqoperators, Oid *ffeqoperators, + int numfkdelsetcols, int16 *fkdelsetcols, + bool old_check_ok, + Oid parentDelTrigger, Oid parentUpdTrigger) +{ + Oid deleteTriggerOid, + updateTriggerOid; + /* * Create the action triggers that enforce the constraint. */ createForeignKeyActionTriggers(rel, RelationGetRelid(pkrel), fkconstraint, - constrOid, indexOid, + parentConstr, indexOid, parentDelTrigger, parentUpdTrigger, &deleteTriggerOid, &updateTriggerOid); @@ -9772,6 +9863,7 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, AttrMap *map; AttrNumber *mapped_pkattnum; Oid partIndexId; + ObjectAddress address; partRel = table_open(pd->oids[i], ShareRowExclusiveLock); @@ -9790,13 +9882,23 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, else mapped_pkattnum = pkattnum; - /* do the deed */ + /* Determine the index to use at this level */ partIndexId = index_get_partition(partRel, indexOid); if (!OidIsValid(partIndexId)) elog(ERROR, "index for %u not found in partition %s", indexOid, RelationGetRelationName(partRel)); + + /* Create entry at this level ... */ + address = addFkConstraint(addFkReferencedSide, + fkconstraint->conname, fkconstraint, rel, + partRel, partIndexId, parentConstr, + numfks, mapped_pkattnum, + fkattnum, pfeqoperators, ppeqoperators, + ffeqoperators, numfkdelsetcols, + fkdelsetcols, true); + /* ... and recurse to our children */ addFkRecurseReferenced(wqueue, fkconstraint, rel, partRel, - partIndexId, constrOid, numfks, + partIndexId, address.objectId, numfks, mapped_pkattnum, fkattnum, pfeqoperators, ppeqoperators, ffeqoperators, numfkdelsetcols, fkdelsetcols, @@ -9812,13 +9914,12 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, } } } - - return address; } /* * addFkRecurseReferencing - * subroutine for ATAddForeignKeyConstraint and CloneFkReferencing + * Recursive helper for the referencing side of foreign key creation, + * which creates the check triggers and recurses * * If the referencing relation is a plain relation, create the necessary check * triggers that implement the constraint, and set up for Phase 3 constraint @@ -9938,10 +10039,7 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, AttrMap *attmap; AttrNumber mapped_fkattnum[INDEX_MAX_KEYS]; bool attached; - char *conname; - Oid constrOid; - ObjectAddress address, - referenced; + ObjectAddress address; ListCell *cell; CheckAlterTableIsSafe(partition); @@ -9983,65 +10081,18 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, /* * No luck finding a good constraint to reuse; create our own. */ - if (ConstraintNameIsUsed(CONSTRAINT_RELATION, - RelationGetRelid(partition), - fkconstraint->conname)) - conname = ChooseConstraintName(RelationGetRelationName(partition), - ChooseForeignKeyConstraintNameAddition(fkconstraint->fk_attrs), - "fkey", - RelationGetNamespace(partition), NIL); - else - conname = fkconstraint->conname; - constrOid = - CreateConstraintEntry(conname, - RelationGetNamespace(partition), - CONSTRAINT_FOREIGN, - fkconstraint->deferrable, - fkconstraint->initdeferred, - fkconstraint->initially_valid, - parentConstr, - partitionId, - mapped_fkattnum, - numfks, - numfks, - InvalidOid, - indexOid, - RelationGetRelid(pkrel), - pkattnum, - pfeqoperators, - ppeqoperators, - ffeqoperators, - numfks, - fkconstraint->fk_upd_action, - fkconstraint->fk_del_action, - fkdelsetcols, - numfkdelsetcols, - fkconstraint->fk_matchtype, - NULL, - NULL, - NULL, - false, - 1, - false, - false); - - /* - * Give this constraint partition-type dependencies on the parent - * constraint as well as the table. - */ - ObjectAddressSet(address, ConstraintRelationId, constrOid); - ObjectAddressSet(referenced, ConstraintRelationId, parentConstr); - recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_PRI); - ObjectAddressSet(referenced, RelationRelationId, partitionId); - recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_SEC); - - /* Make all this visible before recursing */ - CommandCounterIncrement(); + address = addFkConstraint(addFkReferencingSide, + fkconstraint->conname, fkconstraint, + partition, pkrel, indexOid, parentConstr, + numfks, pkattnum, + mapped_fkattnum, pfeqoperators, + ppeqoperators, ffeqoperators, + numfkdelsetcols, fkdelsetcols, true); /* call ourselves to finalize the creation and we're done */ addFkRecurseReferencing(wqueue, fkconstraint, partition, pkrel, indexOid, - constrOid, + address.objectId, numfks, pkattnum, mapped_fkattnum, @@ -10173,6 +10224,7 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) int numfkdelsetcols; AttrNumber confdelsetcols[INDEX_MAX_KEYS]; Constraint *fkconstraint; + ObjectAddress address; Oid deleteTriggerOid, updateTriggerOid; @@ -10206,7 +10258,7 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) * Because we're only expanding the key space at the referenced side, * we don't need to prevent any operation in the referencing table, so * AccessShareLock suffices (assumes that dropping the constraint - * acquires AEL). + * acquires AccessExclusiveLock). */ fkRel = table_open(constrForm->conrelid, AccessShareLock); @@ -10272,12 +10324,20 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) constrForm->confrelid, constrForm->conrelid, &deleteTriggerOid, &updateTriggerOid); + /* Add this constraint ... */ + address = addFkConstraint(addFkReferencedSide, + fkconstraint->conname, fkconstraint, fkRel, + partitionRel, partIndexId, constrOid, + numfks, mapped_confkey, + conkey, conpfeqop, conppeqop, conffeqop, + numfkdelsetcols, confdelsetcols, false); + /* ... and recurse */ addFkRecurseReferenced(NULL, fkconstraint, fkRel, partitionRel, partIndexId, - constrOid, + address.objectId, numfks, mapped_confkey, conkey, @@ -10307,8 +10367,8 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) * child. * * If wqueue is given, it is used to set up phase-3 verification for each - * cloned constraint; if omitted, we assume that such verification is not - * needed (example: the partition is being created anew). + * cloned constraint; omit it if such verification is not needed + * (example: the partition is being created anew). */ static void CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) @@ -10391,9 +10451,7 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) Constraint *fkconstraint; bool attached; Oid indexOid; - Oid constrOid; - ObjectAddress address, - referenced; + ObjectAddress address; ListCell *cell; Oid insertTriggerOid, updateTriggerOid; @@ -10490,7 +10548,7 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) fkconstraint->old_conpfeqop = NIL; fkconstraint->old_pktable_oid = InvalidOid; fkconstraint->skip_validation = false; - fkconstraint->initially_valid = true; + fkconstraint->initially_valid = constrForm->convalidated; for (int i = 0; i < numfks; i++) { Form_pg_attribute att; @@ -10500,71 +10558,29 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) fkconstraint->fk_attrs = lappend(fkconstraint->fk_attrs, makeString(NameStr(att->attname))); } - if (ConstraintNameIsUsed(CONSTRAINT_RELATION, - RelationGetRelid(partRel), - NameStr(constrForm->conname))) - fkconstraint->conname = - ChooseConstraintName(RelationGetRelationName(partRel), - ChooseForeignKeyConstraintNameAddition(fkconstraint->fk_attrs), - "fkey", - RelationGetNamespace(partRel), NIL); - else - fkconstraint->conname = pstrdup(NameStr(constrForm->conname)); indexOid = constrForm->conindid; - constrOid = - CreateConstraintEntry(fkconstraint->conname, - constrForm->connamespace, - CONSTRAINT_FOREIGN, - fkconstraint->deferrable, - fkconstraint->initdeferred, - constrForm->convalidated, - parentConstrOid, - RelationGetRelid(partRel), - mapped_conkey, - numfks, - numfks, - InvalidOid, /* not a domain constraint */ - indexOid, - constrForm->confrelid, /* same foreign rel */ - confkey, - conpfeqop, - conppeqop, - conffeqop, - numfks, - fkconstraint->fk_upd_action, - fkconstraint->fk_del_action, - confdelsetcols, - numfkdelsetcols, - fkconstraint->fk_matchtype, - NULL, - NULL, - NULL, - false, /* islocal */ - 1, /* inhcount */ - false, /* conNoInherit */ - true); - /* Set up partition dependencies for the new constraint */ - ObjectAddressSet(address, ConstraintRelationId, constrOid); - ObjectAddressSet(referenced, ConstraintRelationId, parentConstrOid); - recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_PRI); - ObjectAddressSet(referenced, RelationRelationId, - RelationGetRelid(partRel)); - recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_SEC); + /* Create the pg_constraint entry at this level */ + address = addFkConstraint(addFkReferencingSide, + NameStr(constrForm->conname), fkconstraint, + partRel, pkrel, indexOid, parentConstrOid, + numfks, confkey, + mapped_conkey, conpfeqop, + conppeqop, conffeqop, + numfkdelsetcols, confdelsetcols, + false); /* Done with the cloned constraint's tuple */ ReleaseSysCache(tuple); - /* Make all this visible before recursing */ - CommandCounterIncrement(); - + /* Create the check triggers, and recurse to partitions, if any */ addFkRecurseReferencing(wqueue, fkconstraint, partRel, pkrel, indexOid, - constrOid, + address.objectId, numfks, confkey, mapped_conkey, @@ -10728,6 +10744,81 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk, TriggerSetParentTrigger(trigrel, updateTriggerOid, parentUpdTrigger, partRelid); + /* + * If the referenced table is partitioned, then the partition we're + * attaching now has extra pg_constraint rows and action triggers that are + * no longer needed. Remove those. + */ + if (get_rel_relkind(fk->confrelid) == RELKIND_PARTITIONED_TABLE) + { + Relation pg_constraint = table_open(ConstraintRelationId, RowShareLock); + ObjectAddresses *objs; + HeapTuple consttup; + + ScanKeyInit(&key, + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(fk->conrelid)); + + scan = systable_beginscan(pg_constraint, + ConstraintRelidTypidNameIndexId, + true, NULL, 1, &key); + objs = new_object_addresses(); + while ((consttup = systable_getnext(scan)) != NULL) + { + Form_pg_constraint conform = (Form_pg_constraint) GETSTRUCT(consttup); + + if (conform->conparentid != fk->conoid) + continue; + else + { + ObjectAddress addr; + SysScanDesc scan2; + ScanKeyData key2; + int n PG_USED_FOR_ASSERTS_ONLY; + + ObjectAddressSet(addr, ConstraintRelationId, conform->oid); + add_exact_object_address(&addr, objs); + + /* + * First we must delete the dependency record that binds the + * constraint records together. + */ + n = deleteDependencyRecordsForSpecific(ConstraintRelationId, + conform->oid, + DEPENDENCY_INTERNAL, + ConstraintRelationId, + fk->conoid); + Assert(n == 1); /* actually only one is expected */ + + /* + * Now search for the triggers for this constraint and set + * them up for deletion too + */ + ScanKeyInit(&key2, + Anum_pg_trigger_tgconstraint, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(conform->oid)); + scan2 = systable_beginscan(trigrel, TriggerConstraintIndexId, + true, NULL, 1, &key2); + while ((trigtup = systable_getnext(scan2)) != NULL) + { + ObjectAddressSet(addr, TriggerRelationId, + ((Form_pg_trigger) GETSTRUCT(trigtup))->oid); + add_exact_object_address(&addr, objs); + } + systable_endscan(scan2); + } + } + /* make the dependency deletions visible */ + CommandCounterIncrement(); + performMultipleDeletions(objs, DROP_RESTRICT, + PERFORM_DELETION_INTERNAL); + systable_endscan(scan); + + table_close(pg_constraint, RowShareLock); + } + CommandCounterIncrement(); return true; } @@ -18674,7 +18765,7 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, } /* Do the final part of detaching */ - DetachPartitionFinalize(rel, partRel, concurrent, defaultPartOid); + DetachPartitionFinalize(wqueue, rel, partRel, concurrent, defaultPartOid); ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel)); @@ -18691,8 +18782,8 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, * transaction of the concurrent algorithm fails (crash or abort). */ static void -DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, - Oid defaultPartOid) +DetachPartitionFinalize(List **wqueue, Relation rel, Relation partRel, + bool concurrent, Oid defaultPartOid) { Relation classRel; List *fks; @@ -18728,8 +18819,11 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, { ForeignKeyCacheInfo *fk = lfirst(cell); HeapTuple contup; + HeapTuple parentConTup; Form_pg_constraint conform; + Form_pg_constraint parentConForm; Constraint *fkconstraint; + Oid parentConstrOid; Oid insertTriggerOid, updateTriggerOid; @@ -18746,7 +18840,20 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, continue; } - /* unset conparentid and adjust conislocal, coninhcount, etc. */ + parentConstrOid = conform->conparentid; + + Assert(OidIsValid(conform->conparentid)); + parentConTup = SearchSysCache1(CONSTROID, + ObjectIdGetDatum(parentConstrOid)); + if (!HeapTupleIsValid(parentConTup)) + elog(ERROR, "cache lookup failed for constraint %u", + conform->conparentid); + parentConForm = (Form_pg_constraint) GETSTRUCT(parentConTup); + + /* + * The constraint on this table must be marked no longer a child of + * the parent's constraint, as do its check triggers. + */ ConstraintSetParentConstraint(fk->conoid, InvalidOid, InvalidOid); /* @@ -18764,35 +18871,87 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, RelationGetRelid(partRel)); /* - * Make the action triggers on the referenced relation. When this was - * a partition the action triggers pointed to the parent rel (they - * still do), but now we need separate ones of our own. + * If the referenced side is partitioned (which we know because our + * parent's constraint points to a different relation than ours) then + * we must, in addition to the above, create pg_constraint rows that + * point to each partition, each with its own action triggers. */ - fkconstraint = makeNode(Constraint); - fkconstraint->contype = CONSTRAINT_FOREIGN; - fkconstraint->conname = pstrdup(NameStr(conform->conname)); - fkconstraint->deferrable = conform->condeferrable; - fkconstraint->initdeferred = conform->condeferred; - fkconstraint->location = -1; - fkconstraint->pktable = NULL; - fkconstraint->fk_attrs = NIL; - fkconstraint->pk_attrs = NIL; - fkconstraint->fk_matchtype = conform->confmatchtype; - fkconstraint->fk_upd_action = conform->confupdtype; - fkconstraint->fk_del_action = conform->confdeltype; - fkconstraint->fk_del_set_cols = NIL; - fkconstraint->old_conpfeqop = NIL; - fkconstraint->old_pktable_oid = InvalidOid; - fkconstraint->skip_validation = false; - fkconstraint->initially_valid = true; + if (parentConForm->conrelid != conform->conrelid) + { + int numfks; + AttrNumber conkey[INDEX_MAX_KEYS]; + AttrMap *attmap; + AttrNumber confkey[INDEX_MAX_KEYS]; + Oid conpfeqop[INDEX_MAX_KEYS]; + Oid conppeqop[INDEX_MAX_KEYS]; + Oid conffeqop[INDEX_MAX_KEYS]; + int numfkdelsetcols; + AttrNumber confdelsetcols[INDEX_MAX_KEYS]; + Relation refdRel; - createForeignKeyActionTriggers(partRel, conform->confrelid, - fkconstraint, fk->conoid, - conform->conindid, - InvalidOid, InvalidOid, - NULL, NULL); + DeconstructFkConstraintRow(contup, + &numfks, + conkey, + confkey, + conpfeqop, + conppeqop, + conffeqop, + &numfkdelsetcols, + confdelsetcols); + + /* Create a synthetic node we'll use throughout */ + fkconstraint = makeNode(Constraint); + fkconstraint->contype = CONSTRAINT_FOREIGN; + fkconstraint->conname = pstrdup(NameStr(conform->conname)); + fkconstraint->deferrable = conform->condeferrable; + fkconstraint->initdeferred = conform->condeferred; + fkconstraint->skip_validation = true; + fkconstraint->initially_valid = true; + /* a few irrelevant fields omitted here */ + fkconstraint->pktable = NULL; + fkconstraint->fk_attrs = NIL; + fkconstraint->pk_attrs = NIL; + fkconstraint->fk_matchtype = conform->confmatchtype; + fkconstraint->fk_upd_action = conform->confupdtype; + fkconstraint->fk_del_action = conform->confdeltype; + fkconstraint->fk_del_set_cols = NIL; + fkconstraint->old_conpfeqop = NIL; + fkconstraint->old_pktable_oid = InvalidOid; + fkconstraint->location = -1; + + attmap = build_attrmap_by_name(RelationGetDescr(partRel), + RelationGetDescr(rel)); + for (int i = 0; i < numfks; i++) + { + Form_pg_attribute att; + + att = TupleDescAttr(RelationGetDescr(partRel), + attmap->attnums[conkey[i] - 1] - 1); + fkconstraint->fk_attrs = lappend(fkconstraint->fk_attrs, + makeString(NameStr(att->attname))); + } + + refdRel = table_open(fk->confrelid, AccessShareLock); + + addFkRecurseReferenced(wqueue, fkconstraint, partRel, + refdRel, + conform->conindid, + fk->conoid, + numfks, + confkey, + conkey, + conpfeqop, + conppeqop, + conffeqop, + numfkdelsetcols, + confdelsetcols, + true, + InvalidOid, InvalidOid); + table_close(refdRel, AccessShareLock); + } ReleaseSysCache(contup); + ReleaseSysCache(parentConTup); } list_free_deep(fks); if (trigrel) @@ -18927,7 +19086,7 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, * completion; this completes the detaching process. */ static ObjectAddress -ATExecDetachPartitionFinalize(Relation rel, RangeVar *name) +ATExecDetachPartitionFinalize(List **wqueue, Relation rel, RangeVar *name) { Relation partRel; ObjectAddress address; @@ -18945,7 +19104,7 @@ ATExecDetachPartitionFinalize(Relation rel, RangeVar *name) */ WaitForOlderSnapshots(snap->xmin, false); - DetachPartitionFinalize(rel, partRel, true, InvalidOid); + DetachPartitionFinalize(wqueue, rel, partRel, true, InvalidOid); ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel)); diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 5f4d6f84edf..02cf78ee26a 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -2934,3 +2934,99 @@ DETAIL: drop cascades to table fkpart11.pk drop cascades to table fkpart11.fk_parted drop cascades to table fkpart11.fk_another drop cascades to function fkpart11.print_row() +-- When a table is attached as partition to a partitioned table that has +-- a foreign key to another partitioned table, it acquires a clone of the +-- FK. Upon detach, this clone is not removed, but instead becomes an +-- independent FK. If it then attaches to the partitioned table again, +-- the FK from the parent "takes over" ownership of the independent FK rather +-- than creating a separate one. +CREATE SCHEMA fkpart12 + CREATE TABLE fk_p ( id int, jd int, PRIMARY KEY(id, jd)) PARTITION BY list (id) + CREATE TABLE fk_p_1 PARTITION OF fk_p FOR VALUES IN (1) PARTITION BY list (jd) + CREATE TABLE fk_p_1_1 PARTITION OF fk_p_1 FOR VALUES IN (1) + CREATE TABLE fk_p_1_2 PARTITION OF fk_p_1 FOR VALUES IN (2) + CREATE TABLE fk_p_2 PARTITION OF fk_p FOR VALUES IN (2) PARTITION BY list (jd) + CREATE TABLE fk_p_2_1 PARTITION OF fk_p_2 FOR VALUES IN (1) + CREATE TABLE fk_p_2_2 PARTITION OF fk_p_2 FOR VALUES IN (2) + CREATE TABLE fk_r_1 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) + CREATE TABLE fk_r_2 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) PARTITION BY list (id) + CREATE TABLE fk_r_2_1 PARTITION OF fk_r_2 FOR VALUES IN (2, 1) + CREATE TABLE fk_r ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL, + FOREIGN KEY (p_id, p_jd) REFERENCES fk_p (id, jd) + ) PARTITION BY list (id); +SET search_path TO fkpart12; +INSERT INTO fk_p VALUES (1, 1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2); +\d fk_r_2 + Partitioned table "fkpart12.fk_r_2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + id | integer | | not null | + p_id | integer | | not null | + p_jd | integer | | not null | +Partition of: fk_r FOR VALUES IN (2) +Partition key: LIST (id) +Indexes: + "fk_r_2_pkey" PRIMARY KEY, btree (id) +Foreign-key constraints: + TABLE "fk_r" CONSTRAINT "fk_r_p_id_p_jd_fkey" FOREIGN KEY (p_id, p_jd) REFERENCES fk_p(id, jd) +Number of partitions: 1 (Use \d+ to list them.) + +INSERT INTO fk_r VALUES (1, 1, 1); +INSERT INTO fk_r VALUES (2, 2, 1); +ERROR: insert or update on table "fk_r_2_1" violates foreign key constraint "fk_r_p_id_p_jd_fkey" +DETAIL: Key (p_id, p_jd)=(2, 1) is not present in table "fk_p". +ALTER TABLE fk_r DETACH PARTITION fk_r_1; +ALTER TABLE fk_r DETACH PARTITION fk_r_2; +\d fk_r_2 + Partitioned table "fkpart12.fk_r_2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + id | integer | | not null | + p_id | integer | | not null | + p_jd | integer | | not null | +Partition key: LIST (id) +Indexes: + "fk_r_2_pkey" PRIMARY KEY, btree (id) +Foreign-key constraints: + "fk_r_p_id_p_jd_fkey" FOREIGN KEY (p_id, p_jd) REFERENCES fk_p(id, jd) +Number of partitions: 1 (Use \d+ to list them.) + +INSERT INTO fk_r_1 VALUES (2, 1, 2); -- should fail +ERROR: insert or update on table "fk_r_1" violates foreign key constraint "fk_r_p_id_p_jd_fkey" +DETAIL: Key (p_id, p_jd)=(1, 2) is not present in table "fk_p". +DELETE FROM fk_p; -- should fail +ERROR: update or delete on table "fk_p_1_1" violates foreign key constraint "fk_r_1_p_id_p_jd_fkey1" on table "fk_r_1" +DETAIL: Key (id, jd)=(1, 1) is still referenced from table "fk_r_1". +ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2); +\d fk_r_2 + Partitioned table "fkpart12.fk_r_2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + id | integer | | not null | + p_id | integer | | not null | + p_jd | integer | | not null | +Partition of: fk_r FOR VALUES IN (2) +Partition key: LIST (id) +Indexes: + "fk_r_2_pkey" PRIMARY KEY, btree (id) +Foreign-key constraints: + TABLE "fk_r" CONSTRAINT "fk_r_p_id_p_jd_fkey" FOREIGN KEY (p_id, p_jd) REFERENCES fk_p(id, jd) +Number of partitions: 1 (Use \d+ to list them.) + +DELETE FROM fk_p; -- should fail +ERROR: update or delete on table "fk_p_1_1" violates foreign key constraint "fk_r_p_id_p_jd_fkey2" on table "fk_r" +DETAIL: Key (id, jd)=(1, 1) is still referenced from table "fk_r". +-- these should all fail +ALTER TABLE fk_r_1 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; +ERROR: cannot drop inherited constraint "fk_r_p_id_p_jd_fkey" of relation "fk_r_1" +ALTER TABLE fk_r DROP CONSTRAINT fk_r_p_id_p_jd_fkey1; +ERROR: cannot drop inherited constraint "fk_r_p_id_p_jd_fkey1" of relation "fk_r" +ALTER TABLE fk_r_2 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; +ERROR: cannot drop inherited constraint "fk_r_p_id_p_jd_fkey" of relation "fk_r_2" +SET client_min_messages TO warning; +DROP SCHEMA fkpart12 CASCADE; +RESET client_min_messages; +RESET search_path; diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index bb39c9719a9..9f424f2e143 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -2086,3 +2086,60 @@ UPDATE fkpart11.pk SET a = 3 WHERE a = 4; UPDATE fkpart11.pk SET a = 1 WHERE a = 2; DROP SCHEMA fkpart11 CASCADE; + +-- When a table is attached as partition to a partitioned table that has +-- a foreign key to another partitioned table, it acquires a clone of the +-- FK. Upon detach, this clone is not removed, but instead becomes an +-- independent FK. If it then attaches to the partitioned table again, +-- the FK from the parent "takes over" ownership of the independent FK rather +-- than creating a separate one. +CREATE SCHEMA fkpart12 + CREATE TABLE fk_p ( id int, jd int, PRIMARY KEY(id, jd)) PARTITION BY list (id) + CREATE TABLE fk_p_1 PARTITION OF fk_p FOR VALUES IN (1) PARTITION BY list (jd) + CREATE TABLE fk_p_1_1 PARTITION OF fk_p_1 FOR VALUES IN (1) + CREATE TABLE fk_p_1_2 PARTITION OF fk_p_1 FOR VALUES IN (2) + CREATE TABLE fk_p_2 PARTITION OF fk_p FOR VALUES IN (2) PARTITION BY list (jd) + CREATE TABLE fk_p_2_1 PARTITION OF fk_p_2 FOR VALUES IN (1) + CREATE TABLE fk_p_2_2 PARTITION OF fk_p_2 FOR VALUES IN (2) + CREATE TABLE fk_r_1 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) + CREATE TABLE fk_r_2 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) PARTITION BY list (id) + CREATE TABLE fk_r_2_1 PARTITION OF fk_r_2 FOR VALUES IN (2, 1) + CREATE TABLE fk_r ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL, + FOREIGN KEY (p_id, p_jd) REFERENCES fk_p (id, jd) + ) PARTITION BY list (id); +SET search_path TO fkpart12; + +INSERT INTO fk_p VALUES (1, 1); + +ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2); + +\d fk_r_2 + +INSERT INTO fk_r VALUES (1, 1, 1); +INSERT INTO fk_r VALUES (2, 2, 1); + +ALTER TABLE fk_r DETACH PARTITION fk_r_1; +ALTER TABLE fk_r DETACH PARTITION fk_r_2; + +\d fk_r_2 + +INSERT INTO fk_r_1 VALUES (2, 1, 2); -- should fail +DELETE FROM fk_p; -- should fail + +ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2); + +\d fk_r_2 + +DELETE FROM fk_p; -- should fail + +-- these should all fail +ALTER TABLE fk_r_1 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; +ALTER TABLE fk_r DROP CONSTRAINT fk_r_p_id_p_jd_fkey1; +ALTER TABLE fk_r_2 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; + +SET client_min_messages TO warning; +DROP SCHEMA fkpart12 CASCADE; +RESET client_min_messages; +RESET search_path; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index e4dbc435967..387f2234243 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3075,6 +3075,7 @@ _locale_t _resultmap _stringlist acquireLocksOnSubLinks_context +addFkConstraintSides adjust_appendrel_attrs_context aff_regex_struct allocfunc -- 2.39.5 --mr6adwpq5l3ngngx Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v4_16-0001-Redo-foreign-key-maintenance-during-partition-.patch" From 752bcd855724471abd0dda0a4c9e81ba9ba85009 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= Date: Fri, 18 Oct 2024 16:40:35 +0200 Subject: [PATCH v4_16] Redo foreign key maintenance during partition ATTACH/DETACH ... when the reference table is partitioned. It turns out that the catalog representation we chose for foreign keys connecting partitioned tables is suboptimal, particularly in the sense that a standalone table has a different way to represent it when referencing a partitioned table, than when the same table becomes a partition (and vice versa). This difference means we need to create additional catalog rows on detach (pg_constraint and pg_trigger), and remove them on attach. As a very obvious symptom, we were missing action triggers after detach, which means that you could update/delete rows from the referenced partitioned table that still had referencing rows at the other side previously detached, and fail to throw the required errors. !!!!!!!!! Note that this means existing FKs that refer to partitioned tables might have rows that break relational integrity, on tables that were once partitions. Another possible problem is that trying to reattach a table that had been detached would fail indicating that internal triggers cannot be found, which from the user's point of view is nonsensical. In branches 15 and above, we fix this by creating a new helper function addFkConstraint() which is in charge of creating a standalone pg_constraint row, and repurposing addFkRecurseReferencing() and addFkRecurseReferenced() so that they're only the recursive routine for each side of the FK, and they call addFkConstraint() to create pg_constraint at each partitioning level and add the necessary triggers. These new routines can be used during partition creation, partition attach and detach, and foreign key creation. This reduces redundant code and simplifies the flow. In branches 14 and 13, we have a much simpler fix that consists on simply removing the constraint on detach. The reason is that those branches are missing commit f4566345cf40, which reworked the way this works in a way that we didn't consider back-patchable at the time. I doubt we want to change that now, so this is probably the best we can do there. We opted to leave branch 12 alone, because it's going to end maintenance very soon, and any rushed patch might be worse. In the future we might want to rethink the catalog representation to avoid this problem, but for now the code seems to do what's required to make the constraints operate correctly. Co-authored-by: Jehan-Guillaume de Rorthais Co-authored-by: Tender Wang Co-authored-by: Alvaro Herrera Reported-by: Guillaume Lelarge Reported-by: Jehan-Guillaume de Rorthais Reported-by: Thomas Baehler (SBB CFF FFS) Discussion: https://postgr.es/m/20230420144344.40744130@karst Discussion: https://postgr.es/m/20230705233028.2f554f73@karst Discussion: https://postgr.es/m/GVAP278MB02787E7134FD691861635A8BC9032@GVAP278MB0278.CHEP278.PROD.OUTLOOK.COM Discussion: https://postgr.es/m/18541-628a61bc267cd2d3@postgresql.org --- src/backend/commands/tablecmds.c | 630 ++++++++++++++-------- src/test/regress/expected/foreign_key.out | 96 ++++ src/test/regress/sql/foreign_key.sql | 57 ++ src/tools/pgindent/typedefs.list | 1 + 4 files changed, 549 insertions(+), 235 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 568ad96834e..4a5bb87a12c 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -336,6 +336,14 @@ typedef struct ForeignTruncateInfo List *rels; } ForeignTruncateInfo; +/* Partial or complete FK creation in addFkConstraint() */ +typedef enum addFkConstraintSides +{ + addFkReferencedSide, + addFkReferencingSide, + addFkBothSides, +} addFkConstraintSides; + /* * Partition tables are expected to be dropped when the parent partitioned * table gets dropped. Hence for partitioning we use AUTO dependency. @@ -490,16 +498,25 @@ static ObjectAddress ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo * Relation rel, Constraint *fkconstraint, bool recurse, bool recursing, LOCKMODE lockmode); -static ObjectAddress addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, - Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr, - int numfks, int16 *pkattnum, int16 *fkattnum, - Oid *pfeqoperators, Oid *ppeqoperators, Oid *ffeqoperators, - int numfkdelsetcols, int16 *fkdelsetcols, - bool old_check_ok, - Oid parentDelTrigger, Oid parentUpdTrigger); static void validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums, int numfksetcols, const int16 *fksetcolsattnums, List *fksetcols); +static ObjectAddress addFkConstraint(addFkConstraintSides fkside, + char *constraintname, + Constraint *fkconstraint, Relation rel, + Relation pkrel, Oid indexOid, + Oid parentConstr, + int numfks, int16 *pkattnum, int16 *fkattnum, + Oid *pfeqoperators, Oid *ppeqoperators, + Oid *ffeqoperators, int numfkdelsetcols, + int16 *fkdelsetcols, bool is_internal); +static void addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, + Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr, + int numfks, int16 *pkattnum, int16 *fkattnum, + Oid *pfeqoperators, Oid *ppeqoperators, Oid *ffeqoperators, + int numfkdelsetcols, int16 *fkdelsetcols, + bool old_check_ok, + Oid parentDelTrigger, Oid parentUpdTrigger); static void addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr, int numfks, int16 *pkattnum, int16 *fkattnum, @@ -627,9 +644,11 @@ static void DropClonedTriggersFromPartition(Oid partitionId); static ObjectAddress ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, RangeVar *name, bool concurrent); -static void DetachPartitionFinalize(Relation rel, Relation partRel, - bool concurrent, Oid defaultPartOid); -static ObjectAddress ATExecDetachPartitionFinalize(Relation rel, RangeVar *name); +static void DetachPartitionFinalize(List **wqueue, Relation rel, + Relation partRel, bool concurrent, + Oid defaultPartOid); +static ObjectAddress ATExecDetachPartitionFinalize(List **wqueue, Relation rel, + RangeVar *name); static ObjectAddress ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name); static void validatePartitionedIndex(Relation partedIdx, Relation partedTbl); @@ -5344,7 +5363,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, ((PartitionCmd *) cmd->def)->concurrent); break; case AT_DetachPartitionFinalize: - address = ATExecDetachPartitionFinalize(rel, ((PartitionCmd *) cmd->def)->name); + address = ATExecDetachPartitionFinalize(wqueue, rel, ((PartitionCmd *) cmd->def)->name); break; default: /* oops */ elog(ERROR, "unrecognized alter table type: %d", @@ -9555,25 +9574,37 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, ffeqoperators[i] = ffeqop; } - /* - * Create all the constraint and trigger objects, recursing to partitions - * as necessary. First handle the referenced side. - */ - address = addFkRecurseReferenced(wqueue, fkconstraint, rel, pkrel, - indexOid, - InvalidOid, /* no parent constraint */ - numfks, - pkattnum, - fkattnum, - pfeqoperators, - ppeqoperators, - ffeqoperators, - numfkdelsetcols, - fkdelsetcols, - old_check_ok, - InvalidOid, InvalidOid); + /* First, create the constraint catalog entry itself. */ + address = addFkConstraint(addFkBothSides, + fkconstraint->conname, fkconstraint, rel, pkrel, + indexOid, + InvalidOid, /* no parent constraint */ + numfks, + pkattnum, + fkattnum, + pfeqoperators, + ppeqoperators, + ffeqoperators, + numfkdelsetcols, + fkdelsetcols, + false); - /* Now handle the referencing side. */ + /* Next process the action triggers at the referenced side and recurse */ + addFkRecurseReferenced(wqueue, fkconstraint, rel, pkrel, + indexOid, + address.objectId, + numfks, + pkattnum, + fkattnum, + pfeqoperators, + ppeqoperators, + ffeqoperators, + numfkdelsetcols, + fkdelsetcols, + old_check_ok, + InvalidOid, InvalidOid); + + /* Lastly create the check triggers at the referencing side and recurse */ addFkRecurseReferencing(wqueue, fkconstraint, rel, pkrel, indexOid, address.objectId, @@ -9633,46 +9664,43 @@ validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums, } /* - * addFkRecurseReferenced - * subroutine for ATAddForeignKeyConstraint; recurses on the referenced - * side of the constraint + * addFkConstraint + * Install pg_constraint entries to implement a foreign key constraint. + * Caller must separately invoke addFkRecurseReferenced and + * addFkRecurseReferencing, as appropriate, to install pg_trigger entries + * and (for partitioned tables) recurse to partitions. * - * Create pg_constraint rows for the referenced side of the constraint, - * referencing the parent of the referencing side; also create action triggers - * on leaf partitions. If the table is partitioned, recurse to handle each - * partition. - * - * wqueue is the ALTER TABLE work queue; can be NULL when not running as part - * of an ALTER TABLE sequence. - * fkconstraint is the constraint being added. - * rel is the root referencing relation. - * pkrel is the referenced relation; might be a partition, if recursing. - * indexOid is the OID of the index (on pkrel) implementing this constraint. - * parentConstr is the OID of a parent constraint; InvalidOid if this is a - * top-level constraint. - * numfks is the number of columns in the foreign key - * pkattnum is the attnum array of referenced attributes. - * fkattnum is the attnum array of referencing attributes. - * numfkdelsetcols is the number of columns in the ON DELETE SET NULL/DEFAULT - * (...) clause - * fkdelsetcols is the attnum array of the columns in the ON DELETE SET - * NULL/DEFAULT clause - * pf/pp/ffeqoperators are OID array of operators between columns. - * old_check_ok signals that this constraint replaces an existing one that - * was already validated (thus this one doesn't need validation). - * parentDelTrigger and parentUpdTrigger, when being recursively called on - * a partition, are the OIDs of the parent action triggers for DELETE and - * UPDATE respectively. + * * fkside indicates which side of the FK (or both) to create. Caller must + * call addFkRecurseReferenced if it's addFkReferencedSide, + * addFkRecurseReferencing if it's addFkReferencingSide, or both if it's + * addFkBothSides. + * * constraintname the base name for the constraint being added. This is + * copied to fkconstraint->conname if the latter is not set. + * * fkconstraint is the constraint being added. + * * rel is the root referencing relation. + * * pkrel is the referenced relation; might be a partition, if recursing. + * * indexOid is the OID of the index (on pkrel) implementing this constraint. + * * parentConstr is the OID of a parent constraint; InvalidOid if this is a + * top-level constraint. + * * numfks is the number of columns in the foreign key. + * * pkattnum is the attnum array of referenced attributes. + * * fkattnum is the attnum array of referencing attributes. + * * pf/pp/ffeqoperators are OID array of operators between columns. + * * numfkdelsetcols is the number of columns in the ON DELETE SET NULL/DEFAULT + * (...) clause. + * * fkdelsetcols is the attnum array of the columns in the ON DELETE SET + * NULL/DEFAULT clause. + * * is_internal: whether to mark the constraint as internal (used during + * recursion) */ static ObjectAddress -addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, - Relation pkrel, Oid indexOid, Oid parentConstr, - int numfks, - int16 *pkattnum, int16 *fkattnum, Oid *pfeqoperators, - Oid *ppeqoperators, Oid *ffeqoperators, - int numfkdelsetcols, int16 *fkdelsetcols, - bool old_check_ok, - Oid parentDelTrigger, Oid parentUpdTrigger) +addFkConstraint(addFkConstraintSides fkside, + char *constraintname, Constraint *fkconstraint, + Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr, + int numfks, int16 *pkattnum, + int16 *fkattnum, Oid *pfeqoperators, Oid *ppeqoperators, + Oid *ffeqoperators, int numfkdelsetcols, int16 *fkdelsetcols, + bool is_internal) { ObjectAddress address; Oid constrOid; @@ -9680,8 +9708,6 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, bool conislocal; int coninhcount; bool connoinherit; - Oid deleteTriggerOid, - updateTriggerOid; /* * Verify relkind for each referenced partition. At the top level, this @@ -9700,13 +9726,16 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, */ if (ConstraintNameIsUsed(CONSTRAINT_RELATION, RelationGetRelid(rel), - fkconstraint->conname)) + constraintname)) conname = ChooseConstraintName(RelationGetRelationName(rel), ChooseForeignKeyConstraintNameAddition(fkconstraint->fk_attrs), "fkey", RelationGetNamespace(rel), NIL); else - conname = fkconstraint->conname; + conname = constraintname; + + if (fkconstraint->conname == NULL) + fkconstraint->conname = pstrdup(conname); if (OidIsValid(parentConstr)) { @@ -9758,33 +9787,95 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, conislocal, /* islocal */ coninhcount, /* inhcount */ connoinherit, /* conNoInherit */ - false); /* is_internal */ + is_internal); /* is_internal */ ObjectAddressSet(address, ConstraintRelationId, constrOid); - /* - * Mark the child constraint as part of the parent constraint; it must not - * be dropped on its own. (This constraint is deleted when the partition - * is detached, but a special check needs to occur that the partition - * contains no referenced values.) - */ if (OidIsValid(parentConstr)) { ObjectAddress referenced; ObjectAddressSet(referenced, ConstraintRelationId, parentConstr); - recordDependencyOn(&address, &referenced, DEPENDENCY_INTERNAL); + if (fkside == addFkReferencedSide) + { + /* + * Mark the child constraint as part of the parent constraint; it + * must not be dropped on its own. (This constraint is deleted + * when the partition is detached, but a special check needs to + * occur that the partition contains no referenced values.) + */ + recordDependencyOn(&address, &referenced, DEPENDENCY_INTERNAL); + } + else + { + /* + * Give this constraint partition-type dependencies on the parent + * constraint as well as the table. + */ + recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_PRI); + ObjectAddressSet(referenced, RelationRelationId, RelationGetRelid(rel)); + recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_SEC); + } } /* make new constraint visible, in case we add more */ CommandCounterIncrement(); + return address; +} + +/* + * addFkRecurseReferenced + * Recursive helper for the referenced side of foreign key creation, + * which creates the action triggers and recurses + * + * If the referenced relation is a plain relation, create the necessary action + * triggers that implement the constraint. If the referenced relation is a + * partitioned table, then we create a pg_constraint row referencing the parent + * of the referencing side for it and recurse on this routine for each + * partition. + * + * wqueue is the ALTER TABLE work queue; can be NULL when not running as part + * of an ALTER TABLE sequence. + * fkconstraint is the constraint being added. + * rel is the root referencing relation. + * pkrel is the referenced relation; might be a partition, if recursing. + * indexOid is the OID of the index (on pkrel) implementing this constraint. + * parentConstr is the OID of a parent constraint; InvalidOid if this is a + * top-level constraint. + * numfks is the number of columns in the foreign key + * pkattnum is the attnum array of referenced attributes. + * fkattnum is the attnum array of referencing attributes. + * numfkdelsetcols is the number of columns in the ON DELETE SET NULL/DEFAULT + * (...) clause + * fkdelsetcols is the attnum array of the columns in the ON DELETE SET + * NULL/DEFAULT clause + * pf/pp/ffeqoperators are OID array of operators between columns. + * old_check_ok signals that this constraint replaces an existing one that + * was already validated (thus this one doesn't need validation). + * parentDelTrigger and parentUpdTrigger, when being recursively called on + * a partition, are the OIDs of the parent action triggers for DELETE and + * UPDATE respectively. + */ +static void +addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, + Relation pkrel, Oid indexOid, Oid parentConstr, + int numfks, + int16 *pkattnum, int16 *fkattnum, Oid *pfeqoperators, + Oid *ppeqoperators, Oid *ffeqoperators, + int numfkdelsetcols, int16 *fkdelsetcols, + bool old_check_ok, + Oid parentDelTrigger, Oid parentUpdTrigger) +{ + Oid deleteTriggerOid, + updateTriggerOid; + /* * Create the action triggers that enforce the constraint. */ createForeignKeyActionTriggers(rel, RelationGetRelid(pkrel), fkconstraint, - constrOid, indexOid, + parentConstr, indexOid, parentDelTrigger, parentUpdTrigger, &deleteTriggerOid, &updateTriggerOid); @@ -9803,6 +9894,7 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, AttrMap *map; AttrNumber *mapped_pkattnum; Oid partIndexId; + ObjectAddress address; partRel = table_open(pd->oids[i], ShareRowExclusiveLock); @@ -9822,13 +9914,23 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, else mapped_pkattnum = pkattnum; - /* do the deed */ + /* Determine the index to use at this level */ partIndexId = index_get_partition(partRel, indexOid); if (!OidIsValid(partIndexId)) elog(ERROR, "index for %u not found in partition %s", indexOid, RelationGetRelationName(partRel)); + + /* Create entry at this level ... */ + address = addFkConstraint(addFkReferencedSide, + fkconstraint->conname, fkconstraint, rel, + partRel, partIndexId, parentConstr, + numfks, mapped_pkattnum, + fkattnum, pfeqoperators, ppeqoperators, + ffeqoperators, numfkdelsetcols, + fkdelsetcols, true); + /* ... and recurse to our children */ addFkRecurseReferenced(wqueue, fkconstraint, rel, partRel, - partIndexId, constrOid, numfks, + partIndexId, address.objectId, numfks, mapped_pkattnum, fkattnum, pfeqoperators, ppeqoperators, ffeqoperators, numfkdelsetcols, fkdelsetcols, @@ -9844,13 +9946,12 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, } } } - - return address; } /* * addFkRecurseReferencing - * subroutine for ATAddForeignKeyConstraint and CloneFkReferencing + * Recursive helper for the referencing side of foreign key creation, + * which creates the check triggers and recurses * * If the referencing relation is a plain relation, create the necessary check * triggers that implement the constraint, and set up for Phase 3 constraint @@ -9970,10 +10071,7 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, AttrMap *attmap; AttrNumber mapped_fkattnum[INDEX_MAX_KEYS]; bool attached; - char *conname; - Oid constrOid; - ObjectAddress address, - referenced; + ObjectAddress address; ListCell *cell; CheckAlterTableIsSafe(partition); @@ -10016,65 +10114,18 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, /* * No luck finding a good constraint to reuse; create our own. */ - if (ConstraintNameIsUsed(CONSTRAINT_RELATION, - RelationGetRelid(partition), - fkconstraint->conname)) - conname = ChooseConstraintName(RelationGetRelationName(partition), - ChooseForeignKeyConstraintNameAddition(fkconstraint->fk_attrs), - "fkey", - RelationGetNamespace(partition), NIL); - else - conname = fkconstraint->conname; - constrOid = - CreateConstraintEntry(conname, - RelationGetNamespace(partition), - CONSTRAINT_FOREIGN, - fkconstraint->deferrable, - fkconstraint->initdeferred, - fkconstraint->initially_valid, - parentConstr, - partitionId, - mapped_fkattnum, - numfks, - numfks, - InvalidOid, - indexOid, - RelationGetRelid(pkrel), - pkattnum, - pfeqoperators, - ppeqoperators, - ffeqoperators, - numfks, - fkconstraint->fk_upd_action, - fkconstraint->fk_del_action, - fkdelsetcols, - numfkdelsetcols, - fkconstraint->fk_matchtype, - NULL, - NULL, - NULL, - false, - 1, - false, - false); - - /* - * Give this constraint partition-type dependencies on the parent - * constraint as well as the table. - */ - ObjectAddressSet(address, ConstraintRelationId, constrOid); - ObjectAddressSet(referenced, ConstraintRelationId, parentConstr); - recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_PRI); - ObjectAddressSet(referenced, RelationRelationId, partitionId); - recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_SEC); - - /* Make all this visible before recursing */ - CommandCounterIncrement(); + address = addFkConstraint(addFkReferencingSide, + fkconstraint->conname, fkconstraint, + partition, pkrel, indexOid, parentConstr, + numfks, pkattnum, + mapped_fkattnum, pfeqoperators, + ppeqoperators, ffeqoperators, + numfkdelsetcols, fkdelsetcols, true); /* call ourselves to finalize the creation and we're done */ addFkRecurseReferencing(wqueue, fkconstraint, partition, pkrel, indexOid, - constrOid, + address.objectId, numfks, pkattnum, mapped_fkattnum, @@ -10207,6 +10258,7 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) int numfkdelsetcols; AttrNumber confdelsetcols[INDEX_MAX_KEYS]; Constraint *fkconstraint; + ObjectAddress address; Oid deleteTriggerOid, updateTriggerOid; @@ -10240,7 +10292,7 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) * Because we're only expanding the key space at the referenced side, * we don't need to prevent any operation in the referencing table, so * AccessShareLock suffices (assumes that dropping the constraint - * acquires AEL). + * acquires AccessExclusiveLock). */ fkRel = table_open(constrForm->conrelid, AccessShareLock); @@ -10306,12 +10358,20 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) constrForm->confrelid, constrForm->conrelid, &deleteTriggerOid, &updateTriggerOid); + /* Add this constraint ... */ + address = addFkConstraint(addFkReferencedSide, + fkconstraint->conname, fkconstraint, fkRel, + partitionRel, partIndexId, constrOid, + numfks, mapped_confkey, + conkey, conpfeqop, conppeqop, conffeqop, + numfkdelsetcols, confdelsetcols, false); + /* ... and recurse */ addFkRecurseReferenced(NULL, fkconstraint, fkRel, partitionRel, partIndexId, - constrOid, + address.objectId, numfks, mapped_confkey, conkey, @@ -10341,8 +10401,8 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) * child. * * If wqueue is given, it is used to set up phase-3 verification for each - * cloned constraint; if omitted, we assume that such verification is not - * needed (example: the partition is being created anew). + * cloned constraint; omit it if such verification is not needed + * (example: the partition is being created anew). */ static void CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) @@ -10426,9 +10486,7 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) Constraint *fkconstraint; bool attached; Oid indexOid; - Oid constrOid; - ObjectAddress address, - referenced; + ObjectAddress address; ListCell *lc; Oid insertTriggerOid, updateTriggerOid; @@ -10525,7 +10583,7 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) fkconstraint->old_conpfeqop = NIL; fkconstraint->old_pktable_oid = InvalidOid; fkconstraint->skip_validation = false; - fkconstraint->initially_valid = true; + fkconstraint->initially_valid = constrForm->convalidated; for (int i = 0; i < numfks; i++) { Form_pg_attribute att; @@ -10535,71 +10593,29 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) fkconstraint->fk_attrs = lappend(fkconstraint->fk_attrs, makeString(NameStr(att->attname))); } - if (ConstraintNameIsUsed(CONSTRAINT_RELATION, - RelationGetRelid(partRel), - NameStr(constrForm->conname))) - fkconstraint->conname = - ChooseConstraintName(RelationGetRelationName(partRel), - ChooseForeignKeyConstraintNameAddition(fkconstraint->fk_attrs), - "fkey", - RelationGetNamespace(partRel), NIL); - else - fkconstraint->conname = pstrdup(NameStr(constrForm->conname)); indexOid = constrForm->conindid; - constrOid = - CreateConstraintEntry(fkconstraint->conname, - constrForm->connamespace, - CONSTRAINT_FOREIGN, - fkconstraint->deferrable, - fkconstraint->initdeferred, - constrForm->convalidated, - parentConstrOid, - RelationGetRelid(partRel), - mapped_conkey, - numfks, - numfks, - InvalidOid, /* not a domain constraint */ - indexOid, - constrForm->confrelid, /* same foreign rel */ - confkey, - conpfeqop, - conppeqop, - conffeqop, - numfks, - fkconstraint->fk_upd_action, - fkconstraint->fk_del_action, - confdelsetcols, - numfkdelsetcols, - fkconstraint->fk_matchtype, - NULL, - NULL, - NULL, - false, /* islocal */ - 1, /* inhcount */ - false, /* conNoInherit */ - true); - /* Set up partition dependencies for the new constraint */ - ObjectAddressSet(address, ConstraintRelationId, constrOid); - ObjectAddressSet(referenced, ConstraintRelationId, parentConstrOid); - recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_PRI); - ObjectAddressSet(referenced, RelationRelationId, - RelationGetRelid(partRel)); - recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_SEC); + /* Create the pg_constraint entry at this level */ + address = addFkConstraint(addFkReferencingSide, + NameStr(constrForm->conname), fkconstraint, + partRel, pkrel, indexOid, parentConstrOid, + numfks, confkey, + mapped_conkey, conpfeqop, + conppeqop, conffeqop, + numfkdelsetcols, confdelsetcols, + false); /* Done with the cloned constraint's tuple */ ReleaseSysCache(tuple); - /* Make all this visible before recursing */ - CommandCounterIncrement(); - + /* Create the check triggers, and recurse to partitions, if any */ addFkRecurseReferencing(wqueue, fkconstraint, partRel, pkrel, indexOid, - constrOid, + address.objectId, numfks, confkey, mapped_conkey, @@ -10763,6 +10779,81 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk, TriggerSetParentTrigger(trigrel, updateTriggerOid, parentUpdTrigger, partRelid); + /* + * If the referenced table is partitioned, then the partition we're + * attaching now has extra pg_constraint rows and action triggers that are + * no longer needed. Remove those. + */ + if (get_rel_relkind(fk->confrelid) == RELKIND_PARTITIONED_TABLE) + { + Relation pg_constraint = table_open(ConstraintRelationId, RowShareLock); + ObjectAddresses *objs; + HeapTuple consttup; + + ScanKeyInit(&key, + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(fk->conrelid)); + + scan = systable_beginscan(pg_constraint, + ConstraintRelidTypidNameIndexId, + true, NULL, 1, &key); + objs = new_object_addresses(); + while ((consttup = systable_getnext(scan)) != NULL) + { + Form_pg_constraint conform = (Form_pg_constraint) GETSTRUCT(consttup); + + if (conform->conparentid != fk->conoid) + continue; + else + { + ObjectAddress addr; + SysScanDesc scan2; + ScanKeyData key2; + int n PG_USED_FOR_ASSERTS_ONLY; + + ObjectAddressSet(addr, ConstraintRelationId, conform->oid); + add_exact_object_address(&addr, objs); + + /* + * First we must delete the dependency record that binds the + * constraint records together. + */ + n = deleteDependencyRecordsForSpecific(ConstraintRelationId, + conform->oid, + DEPENDENCY_INTERNAL, + ConstraintRelationId, + fk->conoid); + Assert(n == 1); /* actually only one is expected */ + + /* + * Now search for the triggers for this constraint and set + * them up for deletion too + */ + ScanKeyInit(&key2, + Anum_pg_trigger_tgconstraint, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(conform->oid)); + scan2 = systable_beginscan(trigrel, TriggerConstraintIndexId, + true, NULL, 1, &key2); + while ((trigtup = systable_getnext(scan2)) != NULL) + { + ObjectAddressSet(addr, TriggerRelationId, + ((Form_pg_trigger) GETSTRUCT(trigtup))->oid); + add_exact_object_address(&addr, objs); + } + systable_endscan(scan2); + } + } + /* make the dependency deletions visible */ + CommandCounterIncrement(); + performMultipleDeletions(objs, DROP_RESTRICT, + PERFORM_DELETION_INTERNAL); + systable_endscan(scan); + + table_close(pg_constraint, RowShareLock); + } + CommandCounterIncrement(); return true; } @@ -18653,7 +18744,7 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, } /* Do the final part of detaching */ - DetachPartitionFinalize(rel, partRel, concurrent, defaultPartOid); + DetachPartitionFinalize(wqueue, rel, partRel, concurrent, defaultPartOid); ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel)); @@ -18670,8 +18761,8 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, * transaction of the concurrent algorithm fails (crash or abort). */ static void -DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, - Oid defaultPartOid) +DetachPartitionFinalize(List **wqueue, Relation rel, Relation partRel, + bool concurrent, Oid defaultPartOid) { Relation classRel; List *fks; @@ -18707,8 +18798,11 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, { ForeignKeyCacheInfo *fk = lfirst(cell); HeapTuple contup; + HeapTuple parentConTup; Form_pg_constraint conform; + Form_pg_constraint parentConForm; Constraint *fkconstraint; + Oid parentConstrOid; Oid insertTriggerOid, updateTriggerOid; @@ -18725,7 +18819,20 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, continue; } - /* unset conparentid and adjust conislocal, coninhcount, etc. */ + parentConstrOid = conform->conparentid; + + Assert(OidIsValid(conform->conparentid)); + parentConTup = SearchSysCache1(CONSTROID, + ObjectIdGetDatum(parentConstrOid)); + if (!HeapTupleIsValid(parentConTup)) + elog(ERROR, "cache lookup failed for constraint %u", + conform->conparentid); + parentConForm = (Form_pg_constraint) GETSTRUCT(parentConTup); + + /* + * The constraint on this table must be marked no longer a child of + * the parent's constraint, as do its check triggers. + */ ConstraintSetParentConstraint(fk->conoid, InvalidOid, InvalidOid); /* @@ -18743,35 +18850,88 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, RelationGetRelid(partRel)); /* - * Make the action triggers on the referenced relation. When this was - * a partition the action triggers pointed to the parent rel (they - * still do), but now we need separate ones of our own. + * If the referenced side is partitioned (which we know because our + * parent's constraint points to a different relation than ours) then + * we must, in addition to the above, create pg_constraint rows that + * point to each partition, each with its own action triggers. */ - fkconstraint = makeNode(Constraint); - fkconstraint->contype = CONSTRAINT_FOREIGN; - fkconstraint->conname = pstrdup(NameStr(conform->conname)); - fkconstraint->deferrable = conform->condeferrable; - fkconstraint->initdeferred = conform->condeferred; - fkconstraint->location = -1; - fkconstraint->pktable = NULL; - fkconstraint->fk_attrs = NIL; - fkconstraint->pk_attrs = NIL; - fkconstraint->fk_matchtype = conform->confmatchtype; - fkconstraint->fk_upd_action = conform->confupdtype; - fkconstraint->fk_del_action = conform->confdeltype; - fkconstraint->fk_del_set_cols = NIL; - fkconstraint->old_conpfeqop = NIL; - fkconstraint->old_pktable_oid = InvalidOid; - fkconstraint->skip_validation = false; - fkconstraint->initially_valid = true; + if (parentConForm->conrelid != conform->conrelid) + { + int numfks; + AttrNumber conkey[INDEX_MAX_KEYS]; + AttrMap *attmap; + AttrNumber confkey[INDEX_MAX_KEYS]; + Oid conpfeqop[INDEX_MAX_KEYS]; + Oid conppeqop[INDEX_MAX_KEYS]; + Oid conffeqop[INDEX_MAX_KEYS]; + int numfkdelsetcols; + AttrNumber confdelsetcols[INDEX_MAX_KEYS]; + Relation refdRel; - createForeignKeyActionTriggers(partRel, conform->confrelid, - fkconstraint, fk->conoid, - conform->conindid, - InvalidOid, InvalidOid, - NULL, NULL); + DeconstructFkConstraintRow(contup, + &numfks, + conkey, + confkey, + conpfeqop, + conppeqop, + conffeqop, + &numfkdelsetcols, + confdelsetcols); + + /* Create a synthetic node we'll use throughout */ + fkconstraint = makeNode(Constraint); + fkconstraint->contype = CONSTRAINT_FOREIGN; + fkconstraint->conname = pstrdup(NameStr(conform->conname)); + fkconstraint->deferrable = conform->condeferrable; + fkconstraint->initdeferred = conform->condeferred; + fkconstraint->skip_validation = true; + fkconstraint->initially_valid = true; + /* a few irrelevant fields omitted here */ + fkconstraint->pktable = NULL; + fkconstraint->fk_attrs = NIL; + fkconstraint->pk_attrs = NIL; + fkconstraint->fk_matchtype = conform->confmatchtype; + fkconstraint->fk_upd_action = conform->confupdtype; + fkconstraint->fk_del_action = conform->confdeltype; + fkconstraint->fk_del_set_cols = NIL; + fkconstraint->old_conpfeqop = NIL; + fkconstraint->old_pktable_oid = InvalidOid; + fkconstraint->location = -1; + + attmap = build_attrmap_by_name(RelationGetDescr(partRel), + RelationGetDescr(rel), + false); + for (int i = 0; i < numfks; i++) + { + Form_pg_attribute att; + + att = TupleDescAttr(RelationGetDescr(partRel), + attmap->attnums[conkey[i] - 1] - 1); + fkconstraint->fk_attrs = lappend(fkconstraint->fk_attrs, + makeString(NameStr(att->attname))); + } + + refdRel = table_open(fk->confrelid, AccessShareLock); + + addFkRecurseReferenced(wqueue, fkconstraint, partRel, + refdRel, + conform->conindid, + fk->conoid, + numfks, + confkey, + conkey, + conpfeqop, + conppeqop, + conffeqop, + numfkdelsetcols, + confdelsetcols, + true, + InvalidOid, InvalidOid); + table_close(refdRel, AccessShareLock); + } ReleaseSysCache(contup); + ReleaseSysCache(parentConTup); } list_free_deep(fks); if (trigrel) @@ -18906,7 +19066,7 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, * completion; this completes the detaching process. */ static ObjectAddress -ATExecDetachPartitionFinalize(Relation rel, RangeVar *name) +ATExecDetachPartitionFinalize(List **wqueue, Relation rel, RangeVar *name) { Relation partRel; ObjectAddress address; @@ -18924,7 +19084,7 @@ ATExecDetachPartitionFinalize(Relation rel, RangeVar *name) */ WaitForOlderSnapshots(snap->xmin, false); - DetachPartitionFinalize(rel, partRel, true, InvalidOid); + DetachPartitionFinalize(wqueue, rel, partRel, true, InvalidOid); ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel)); diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 037b73204ab..b6e8d602f9d 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -2934,3 +2934,99 @@ DETAIL: drop cascades to table fkpart11.pk drop cascades to table fkpart11.fk_parted drop cascades to table fkpart11.fk_another drop cascades to function fkpart11.print_row() +-- When a table is attached as partition to a partitioned table that has +-- a foreign key to another partitioned table, it acquires a clone of the +-- FK. Upon detach, this clone is not removed, but instead becomes an +-- independent FK. If it then attaches to the partitioned table again, +-- the FK from the parent "takes over" ownership of the independent FK rather +-- than creating a separate one. +CREATE SCHEMA fkpart12 + CREATE TABLE fk_p ( id int, jd int, PRIMARY KEY(id, jd)) PARTITION BY list (id) + CREATE TABLE fk_p_1 PARTITION OF fk_p FOR VALUES IN (1) PARTITION BY list (jd) + CREATE TABLE fk_p_1_1 PARTITION OF fk_p_1 FOR VALUES IN (1) + CREATE TABLE fk_p_1_2 PARTITION OF fk_p_1 FOR VALUES IN (2) + CREATE TABLE fk_p_2 PARTITION OF fk_p FOR VALUES IN (2) PARTITION BY list (jd) + CREATE TABLE fk_p_2_1 PARTITION OF fk_p_2 FOR VALUES IN (1) + CREATE TABLE fk_p_2_2 PARTITION OF fk_p_2 FOR VALUES IN (2) + CREATE TABLE fk_r_1 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) + CREATE TABLE fk_r_2 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) PARTITION BY list (id) + CREATE TABLE fk_r_2_1 PARTITION OF fk_r_2 FOR VALUES IN (2, 1) + CREATE TABLE fk_r ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL, + FOREIGN KEY (p_id, p_jd) REFERENCES fk_p (id, jd) + ) PARTITION BY list (id); +SET search_path TO fkpart12; +INSERT INTO fk_p VALUES (1, 1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2); +\d fk_r_2 + Partitioned table "fkpart12.fk_r_2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + id | integer | | not null | + p_id | integer | | not null | + p_jd | integer | | not null | +Partition of: fk_r FOR VALUES IN (2) +Partition key: LIST (id) +Indexes: + "fk_r_2_pkey" PRIMARY KEY, btree (id) +Foreign-key constraints: + TABLE "fk_r" CONSTRAINT "fk_r_p_id_p_jd_fkey" FOREIGN KEY (p_id, p_jd) REFERENCES fk_p(id, jd) +Number of partitions: 1 (Use \d+ to list them.) + +INSERT INTO fk_r VALUES (1, 1, 1); +INSERT INTO fk_r VALUES (2, 2, 1); +ERROR: insert or update on table "fk_r_2_1" violates foreign key constraint "fk_r_p_id_p_jd_fkey" +DETAIL: Key (p_id, p_jd)=(2, 1) is not present in table "fk_p". +ALTER TABLE fk_r DETACH PARTITION fk_r_1; +ALTER TABLE fk_r DETACH PARTITION fk_r_2; +\d fk_r_2 + Partitioned table "fkpart12.fk_r_2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + id | integer | | not null | + p_id | integer | | not null | + p_jd | integer | | not null | +Partition key: LIST (id) +Indexes: + "fk_r_2_pkey" PRIMARY KEY, btree (id) +Foreign-key constraints: + "fk_r_p_id_p_jd_fkey" FOREIGN KEY (p_id, p_jd) REFERENCES fk_p(id, jd) +Number of partitions: 1 (Use \d+ to list them.) + +INSERT INTO fk_r_1 VALUES (2, 1, 2); -- should fail +ERROR: insert or update on table "fk_r_1" violates foreign key constraint "fk_r_p_id_p_jd_fkey" +DETAIL: Key (p_id, p_jd)=(1, 2) is not present in table "fk_p". +DELETE FROM fk_p; -- should fail +ERROR: update or delete on table "fk_p_1_1" violates foreign key constraint "fk_r_1_p_id_p_jd_fkey1" on table "fk_r_1" +DETAIL: Key (id, jd)=(1, 1) is still referenced from table "fk_r_1". +ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2); +\d fk_r_2 + Partitioned table "fkpart12.fk_r_2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + id | integer | | not null | + p_id | integer | | not null | + p_jd | integer | | not null | +Partition of: fk_r FOR VALUES IN (2) +Partition key: LIST (id) +Indexes: + "fk_r_2_pkey" PRIMARY KEY, btree (id) +Foreign-key constraints: + TABLE "fk_r" CONSTRAINT "fk_r_p_id_p_jd_fkey" FOREIGN KEY (p_id, p_jd) REFERENCES fk_p(id, jd) +Number of partitions: 1 (Use \d+ to list them.) + +DELETE FROM fk_p; -- should fail +ERROR: update or delete on table "fk_p_1_1" violates foreign key constraint "fk_r_p_id_p_jd_fkey2" on table "fk_r" +DETAIL: Key (id, jd)=(1, 1) is still referenced from table "fk_r". +-- these should all fail +ALTER TABLE fk_r_1 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; +ERROR: cannot drop inherited constraint "fk_r_p_id_p_jd_fkey" of relation "fk_r_1" +ALTER TABLE fk_r DROP CONSTRAINT fk_r_p_id_p_jd_fkey1; +ERROR: cannot drop inherited constraint "fk_r_p_id_p_jd_fkey1" of relation "fk_r" +ALTER TABLE fk_r_2 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; +ERROR: cannot drop inherited constraint "fk_r_p_id_p_jd_fkey" of relation "fk_r_2" +SET client_min_messages TO warning; +DROP SCHEMA fkpart12 CASCADE; +RESET client_min_messages; +RESET search_path; diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index 9a547fb1be0..09b089b60e1 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -2086,3 +2086,60 @@ UPDATE fkpart11.pk SET a = 3 WHERE a = 4; UPDATE fkpart11.pk SET a = 1 WHERE a = 2; DROP SCHEMA fkpart11 CASCADE; + +-- When a table is attached as partition to a partitioned table that has +-- a foreign key to another partitioned table, it acquires a clone of the +-- FK. Upon detach, this clone is not removed, but instead becomes an +-- independent FK. If it then attaches to the partitioned table again, +-- the FK from the parent "takes over" ownership of the independent FK rather +-- than creating a separate one. +CREATE SCHEMA fkpart12 + CREATE TABLE fk_p ( id int, jd int, PRIMARY KEY(id, jd)) PARTITION BY list (id) + CREATE TABLE fk_p_1 PARTITION OF fk_p FOR VALUES IN (1) PARTITION BY list (jd) + CREATE TABLE fk_p_1_1 PARTITION OF fk_p_1 FOR VALUES IN (1) + CREATE TABLE fk_p_1_2 PARTITION OF fk_p_1 FOR VALUES IN (2) + CREATE TABLE fk_p_2 PARTITION OF fk_p FOR VALUES IN (2) PARTITION BY list (jd) + CREATE TABLE fk_p_2_1 PARTITION OF fk_p_2 FOR VALUES IN (1) + CREATE TABLE fk_p_2_2 PARTITION OF fk_p_2 FOR VALUES IN (2) + CREATE TABLE fk_r_1 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) + CREATE TABLE fk_r_2 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) PARTITION BY list (id) + CREATE TABLE fk_r_2_1 PARTITION OF fk_r_2 FOR VALUES IN (2, 1) + CREATE TABLE fk_r ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL, + FOREIGN KEY (p_id, p_jd) REFERENCES fk_p (id, jd) + ) PARTITION BY list (id); +SET search_path TO fkpart12; + +INSERT INTO fk_p VALUES (1, 1); + +ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2); + +\d fk_r_2 + +INSERT INTO fk_r VALUES (1, 1, 1); +INSERT INTO fk_r VALUES (2, 2, 1); + +ALTER TABLE fk_r DETACH PARTITION fk_r_1; +ALTER TABLE fk_r DETACH PARTITION fk_r_2; + +\d fk_r_2 + +INSERT INTO fk_r_1 VALUES (2, 1, 2); -- should fail +DELETE FROM fk_p; -- should fail + +ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2); + +\d fk_r_2 + +DELETE FROM fk_p; -- should fail + +-- these should all fail +ALTER TABLE fk_r_1 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; +ALTER TABLE fk_r DROP CONSTRAINT fk_r_p_id_p_jd_fkey1; +ALTER TABLE fk_r_2 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; + +SET client_min_messages TO warning; +DROP SCHEMA fkpart12 CASCADE; +RESET client_min_messages; +RESET search_path; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 4791528e140..43570438e99 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3130,6 +3130,7 @@ _resultmap _stringlist acquireLocksOnSubLinks_context add_nulling_relids_context +addFkConstraintSides adjust_appendrel_attrs_context allocfunc amadjustmembers_function -- 2.39.5 --mr6adwpq5l3ngngx Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v4_17-0001-Redo-foreign-key-maintenance-during-partition-.patch" From 54842333a6c0fab92dea6422529fb928ebae3877 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= Date: Fri, 18 Oct 2024 16:40:35 +0200 Subject: [PATCH v4_17] Redo foreign key maintenance during partition ATTACH/DETACH ... when the reference table is partitioned. It turns out that the catalog representation we chose for foreign keys connecting partitioned tables is suboptimal, particularly in the sense that a standalone table has a different way to represent it when referencing a partitioned table, than when the same table becomes a partition (and vice versa). This difference means we need to create additional catalog rows on detach (pg_constraint and pg_trigger), and remove them on attach. As a very obvious symptom, we were missing action triggers after detach, which means that you could update/delete rows from the referenced partitioned table that still had referencing rows at the other side previously detached, and fail to throw the required errors. !!!!!!!!! Note that this means existing FKs that refer to partitioned tables might have rows that break relational integrity, on tables that were once partitions. Another possible problem is that trying to reattach a table that had been detached would fail indicating that internal triggers cannot be found, which from the user's point of view is nonsensical. In branches 15 and above, we fix this by creating a new helper function addFkConstraint() which is in charge of creating a standalone pg_constraint row, and repurposing addFkRecurseReferencing() and addFkRecurseReferenced() so that they're only the recursive routine for each side of the FK, and they call addFkConstraint() to create pg_constraint at each partitioning level and add the necessary triggers. These new routines can be used during partition creation, partition attach and detach, and foreign key creation. This reduces redundant code and simplifies the flow. In branches 14 and 13, we have a much simpler fix that consists on simply removing the constraint on detach. The reason is that those branches are missing commit f4566345cf40, which reworked the way this works in a way that we didn't consider back-patchable at the time. I doubt we want to change that now, so this is probably the best we can do there. We opted to leave branch 12 alone, because it's going to end maintenance very soon, and any rushed patch might be worse. In the future we might want to rethink the catalog representation to avoid this problem, but for now the code seems to do what's required to make the constraints operate correctly. Co-authored-by: Jehan-Guillaume de Rorthais Co-authored-by: Tender Wang Co-authored-by: Alvaro Herrera Reported-by: Guillaume Lelarge Reported-by: Jehan-Guillaume de Rorthais Reported-by: Thomas Baehler (SBB CFF FFS) Discussion: https://postgr.es/m/20230420144344.40744130@karst Discussion: https://postgr.es/m/20230705233028.2f554f73@karst Discussion: https://postgr.es/m/GVAP278MB02787E7134FD691861635A8BC9032@GVAP278MB0278.CHEP278.PROD.OUTLOOK.COM Discussion: https://postgr.es/m/18541-628a61bc267cd2d3@postgresql.org --- src/backend/commands/tablecmds.c | 630 ++++++++++++++-------- src/test/regress/expected/foreign_key.out | 96 ++++ src/test/regress/sql/foreign_key.sql | 57 ++ src/tools/pgindent/typedefs.list | 1 + 4 files changed, 549 insertions(+), 235 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 0ad0dfbcc57..c9ccdae1187 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -345,6 +345,14 @@ typedef struct ForeignTruncateInfo List *rels; } ForeignTruncateInfo; +/* Partial or complete FK creation in addFkConstraint() */ +typedef enum addFkConstraintSides +{ + addFkReferencedSide, + addFkReferencingSide, + addFkBothSides, +} addFkConstraintSides; + /* * Partition tables are expected to be dropped when the parent partitioned * table gets dropped. Hence for partitioning we use AUTO dependency. @@ -504,16 +512,25 @@ static ObjectAddress ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo * Relation rel, Constraint *fkconstraint, bool recurse, bool recursing, LOCKMODE lockmode); -static ObjectAddress addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, - Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr, - int numfks, int16 *pkattnum, int16 *fkattnum, - Oid *pfeqoperators, Oid *ppeqoperators, Oid *ffeqoperators, - int numfkdelsetcols, int16 *fkdelsetcols, - bool old_check_ok, - Oid parentDelTrigger, Oid parentUpdTrigger); static void validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums, int numfksetcols, const int16 *fksetcolsattnums, List *fksetcols); +static ObjectAddress addFkConstraint(addFkConstraintSides fkside, + char *constraintname, + Constraint *fkconstraint, Relation rel, + Relation pkrel, Oid indexOid, + Oid parentConstr, + int numfks, int16 *pkattnum, int16 *fkattnum, + Oid *pfeqoperators, Oid *ppeqoperators, + Oid *ffeqoperators, int numfkdelsetcols, + int16 *fkdelsetcols, bool is_internal); +static void addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, + Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr, + int numfks, int16 *pkattnum, int16 *fkattnum, + Oid *pfeqoperators, Oid *ppeqoperators, Oid *ffeqoperators, + int numfkdelsetcols, int16 *fkdelsetcols, + bool old_check_ok, + Oid parentDelTrigger, Oid parentUpdTrigger); static void addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr, int numfks, int16 *pkattnum, int16 *fkattnum, @@ -644,9 +661,11 @@ static void DropClonedTriggersFromPartition(Oid partitionId); static ObjectAddress ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, RangeVar *name, bool concurrent); -static void DetachPartitionFinalize(Relation rel, Relation partRel, - bool concurrent, Oid defaultPartOid); -static ObjectAddress ATExecDetachPartitionFinalize(Relation rel, RangeVar *name); +static void DetachPartitionFinalize(List **wqueue, Relation rel, + Relation partRel, bool concurrent, + Oid defaultPartOid); +static ObjectAddress ATExecDetachPartitionFinalize(List **wqueue, Relation rel, + RangeVar *name); static ObjectAddress ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name); static void validatePartitionedIndex(Relation partedIdx, Relation partedTbl); @@ -5512,7 +5531,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, ((PartitionCmd *) cmd->def)->concurrent); break; case AT_DetachPartitionFinalize: - address = ATExecDetachPartitionFinalize(rel, ((PartitionCmd *) cmd->def)->name); + address = ATExecDetachPartitionFinalize(wqueue, rel, ((PartitionCmd *) cmd->def)->name); break; default: /* oops */ elog(ERROR, "unrecognized alter table type: %d", @@ -9931,25 +9950,37 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, ffeqoperators[i] = ffeqop; } - /* - * Create all the constraint and trigger objects, recursing to partitions - * as necessary. First handle the referenced side. - */ - address = addFkRecurseReferenced(wqueue, fkconstraint, rel, pkrel, - indexOid, - InvalidOid, /* no parent constraint */ - numfks, - pkattnum, - fkattnum, - pfeqoperators, - ppeqoperators, - ffeqoperators, - numfkdelsetcols, - fkdelsetcols, - old_check_ok, - InvalidOid, InvalidOid); + /* First, create the constraint catalog entry itself. */ + address = addFkConstraint(addFkBothSides, + fkconstraint->conname, fkconstraint, rel, pkrel, + indexOid, + InvalidOid, /* no parent constraint */ + numfks, + pkattnum, + fkattnum, + pfeqoperators, + ppeqoperators, + ffeqoperators, + numfkdelsetcols, + fkdelsetcols, + false); - /* Now handle the referencing side. */ + /* Next process the action triggers at the referenced side and recurse */ + addFkRecurseReferenced(wqueue, fkconstraint, rel, pkrel, + indexOid, + address.objectId, + numfks, + pkattnum, + fkattnum, + pfeqoperators, + ppeqoperators, + ffeqoperators, + numfkdelsetcols, + fkdelsetcols, + old_check_ok, + InvalidOid, InvalidOid); + + /* Lastly create the check triggers at the referencing side and recurse */ addFkRecurseReferencing(wqueue, fkconstraint, rel, pkrel, indexOid, address.objectId, @@ -10009,46 +10040,43 @@ validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums, } /* - * addFkRecurseReferenced - * subroutine for ATAddForeignKeyConstraint; recurses on the referenced - * side of the constraint + * addFkConstraint + * Install pg_constraint entries to implement a foreign key constraint. + * Caller must separately invoke addFkRecurseReferenced and + * addFkRecurseReferencing, as appropriate, to install pg_trigger entries + * and (for partitioned tables) recurse to partitions. * - * Create pg_constraint rows for the referenced side of the constraint, - * referencing the parent of the referencing side; also create action triggers - * on leaf partitions. If the table is partitioned, recurse to handle each - * partition. - * - * wqueue is the ALTER TABLE work queue; can be NULL when not running as part - * of an ALTER TABLE sequence. - * fkconstraint is the constraint being added. - * rel is the root referencing relation. - * pkrel is the referenced relation; might be a partition, if recursing. - * indexOid is the OID of the index (on pkrel) implementing this constraint. - * parentConstr is the OID of a parent constraint; InvalidOid if this is a - * top-level constraint. - * numfks is the number of columns in the foreign key - * pkattnum is the attnum array of referenced attributes. - * fkattnum is the attnum array of referencing attributes. - * numfkdelsetcols is the number of columns in the ON DELETE SET NULL/DEFAULT - * (...) clause - * fkdelsetcols is the attnum array of the columns in the ON DELETE SET - * NULL/DEFAULT clause - * pf/pp/ffeqoperators are OID array of operators between columns. - * old_check_ok signals that this constraint replaces an existing one that - * was already validated (thus this one doesn't need validation). - * parentDelTrigger and parentUpdTrigger, when being recursively called on - * a partition, are the OIDs of the parent action triggers for DELETE and - * UPDATE respectively. + * * fkside indicates which side of the FK (or both) to create. Caller must + * call addFkRecurseReferenced if it's addFkReferencedSide, + * addFkRecurseReferencing if it's addFkReferencingSide, or both if it's + * addFkBothSides. + * * constraintname the base name for the constraint being added. This is + * copied to fkconstraint->conname if the latter is not set. + * * fkconstraint is the constraint being added. + * * rel is the root referencing relation. + * * pkrel is the referenced relation; might be a partition, if recursing. + * * indexOid is the OID of the index (on pkrel) implementing this constraint. + * * parentConstr is the OID of a parent constraint; InvalidOid if this is a + * top-level constraint. + * * numfks is the number of columns in the foreign key. + * * pkattnum is the attnum array of referenced attributes. + * * fkattnum is the attnum array of referencing attributes. + * * pf/pp/ffeqoperators are OID array of operators between columns. + * * numfkdelsetcols is the number of columns in the ON DELETE SET NULL/DEFAULT + * (...) clause. + * * fkdelsetcols is the attnum array of the columns in the ON DELETE SET + * NULL/DEFAULT clause. + * * is_internal: whether to mark the constraint as internal (used during + * recursion) */ static ObjectAddress -addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, - Relation pkrel, Oid indexOid, Oid parentConstr, - int numfks, - int16 *pkattnum, int16 *fkattnum, Oid *pfeqoperators, - Oid *ppeqoperators, Oid *ffeqoperators, - int numfkdelsetcols, int16 *fkdelsetcols, - bool old_check_ok, - Oid parentDelTrigger, Oid parentUpdTrigger) +addFkConstraint(addFkConstraintSides fkside, + char *constraintname, Constraint *fkconstraint, + Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr, + int numfks, int16 *pkattnum, + int16 *fkattnum, Oid *pfeqoperators, Oid *ppeqoperators, + Oid *ffeqoperators, int numfkdelsetcols, int16 *fkdelsetcols, + bool is_internal) { ObjectAddress address; Oid constrOid; @@ -10056,8 +10084,6 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, bool conislocal; int coninhcount; bool connoinherit; - Oid deleteTriggerOid, - updateTriggerOid; /* * Verify relkind for each referenced partition. At the top level, this @@ -10076,13 +10102,16 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, */ if (ConstraintNameIsUsed(CONSTRAINT_RELATION, RelationGetRelid(rel), - fkconstraint->conname)) + constraintname)) conname = ChooseConstraintName(RelationGetRelationName(rel), ChooseForeignKeyConstraintNameAddition(fkconstraint->fk_attrs), "fkey", RelationGetNamespace(rel), NIL); else - conname = fkconstraint->conname; + conname = constraintname; + + if (fkconstraint->conname == NULL) + fkconstraint->conname = pstrdup(conname); if (OidIsValid(parentConstr)) { @@ -10134,33 +10163,95 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, conislocal, /* islocal */ coninhcount, /* inhcount */ connoinherit, /* conNoInherit */ - false); /* is_internal */ + is_internal); /* is_internal */ ObjectAddressSet(address, ConstraintRelationId, constrOid); - /* - * Mark the child constraint as part of the parent constraint; it must not - * be dropped on its own. (This constraint is deleted when the partition - * is detached, but a special check needs to occur that the partition - * contains no referenced values.) - */ if (OidIsValid(parentConstr)) { ObjectAddress referenced; ObjectAddressSet(referenced, ConstraintRelationId, parentConstr); - recordDependencyOn(&address, &referenced, DEPENDENCY_INTERNAL); + if (fkside == addFkReferencedSide) + { + /* + * Mark the child constraint as part of the parent constraint; it + * must not be dropped on its own. (This constraint is deleted + * when the partition is detached, but a special check needs to + * occur that the partition contains no referenced values.) + */ + recordDependencyOn(&address, &referenced, DEPENDENCY_INTERNAL); + } + else + { + /* + * Give this constraint partition-type dependencies on the parent + * constraint as well as the table. + */ + recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_PRI); + ObjectAddressSet(referenced, RelationRelationId, RelationGetRelid(rel)); + recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_SEC); + } } /* make new constraint visible, in case we add more */ CommandCounterIncrement(); + return address; +} + +/* + * addFkRecurseReferenced + * Recursive helper for the referenced side of foreign key creation, + * which creates the action triggers and recurses + * + * If the referenced relation is a plain relation, create the necessary action + * triggers that implement the constraint. If the referenced relation is a + * partitioned table, then we create a pg_constraint row referencing the parent + * of the referencing side for it and recurse on this routine for each + * partition. + * + * wqueue is the ALTER TABLE work queue; can be NULL when not running as part + * of an ALTER TABLE sequence. + * fkconstraint is the constraint being added. + * rel is the root referencing relation. + * pkrel is the referenced relation; might be a partition, if recursing. + * indexOid is the OID of the index (on pkrel) implementing this constraint. + * parentConstr is the OID of a parent constraint; InvalidOid if this is a + * top-level constraint. + * numfks is the number of columns in the foreign key + * pkattnum is the attnum array of referenced attributes. + * fkattnum is the attnum array of referencing attributes. + * numfkdelsetcols is the number of columns in the ON DELETE SET NULL/DEFAULT + * (...) clause + * fkdelsetcols is the attnum array of the columns in the ON DELETE SET + * NULL/DEFAULT clause + * pf/pp/ffeqoperators are OID array of operators between columns. + * old_check_ok signals that this constraint replaces an existing one that + * was already validated (thus this one doesn't need validation). + * parentDelTrigger and parentUpdTrigger, when being recursively called on + * a partition, are the OIDs of the parent action triggers for DELETE and + * UPDATE respectively. + */ +static void +addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, + Relation pkrel, Oid indexOid, Oid parentConstr, + int numfks, + int16 *pkattnum, int16 *fkattnum, Oid *pfeqoperators, + Oid *ppeqoperators, Oid *ffeqoperators, + int numfkdelsetcols, int16 *fkdelsetcols, + bool old_check_ok, + Oid parentDelTrigger, Oid parentUpdTrigger) +{ + Oid deleteTriggerOid, + updateTriggerOid; + /* * Create the action triggers that enforce the constraint. */ createForeignKeyActionTriggers(rel, RelationGetRelid(pkrel), fkconstraint, - constrOid, indexOid, + parentConstr, indexOid, parentDelTrigger, parentUpdTrigger, &deleteTriggerOid, &updateTriggerOid); @@ -10179,6 +10270,7 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, AttrMap *map; AttrNumber *mapped_pkattnum; Oid partIndexId; + ObjectAddress address; partRel = table_open(pd->oids[i], ShareRowExclusiveLock); @@ -10198,13 +10290,23 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, else mapped_pkattnum = pkattnum; - /* do the deed */ + /* Determine the index to use at this level */ partIndexId = index_get_partition(partRel, indexOid); if (!OidIsValid(partIndexId)) elog(ERROR, "index for %u not found in partition %s", indexOid, RelationGetRelationName(partRel)); + + /* Create entry at this level ... */ + address = addFkConstraint(addFkReferencedSide, + fkconstraint->conname, fkconstraint, rel, + partRel, partIndexId, parentConstr, + numfks, mapped_pkattnum, + fkattnum, pfeqoperators, ppeqoperators, + ffeqoperators, numfkdelsetcols, + fkdelsetcols, true); + /* ... and recurse to our children */ addFkRecurseReferenced(wqueue, fkconstraint, rel, partRel, - partIndexId, constrOid, numfks, + partIndexId, address.objectId, numfks, mapped_pkattnum, fkattnum, pfeqoperators, ppeqoperators, ffeqoperators, numfkdelsetcols, fkdelsetcols, @@ -10220,13 +10322,12 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, } } } - - return address; } /* * addFkRecurseReferencing - * subroutine for ATAddForeignKeyConstraint and CloneFkReferencing + * Recursive helper for the referencing side of foreign key creation, + * which creates the check triggers and recurses * * If the referencing relation is a plain relation, create the necessary check * triggers that implement the constraint, and set up for Phase 3 constraint @@ -10346,10 +10447,7 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, AttrMap *attmap; AttrNumber mapped_fkattnum[INDEX_MAX_KEYS]; bool attached; - char *conname; - Oid constrOid; - ObjectAddress address, - referenced; + ObjectAddress address; ListCell *cell; CheckAlterTableIsSafe(partition); @@ -10392,65 +10490,18 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, /* * No luck finding a good constraint to reuse; create our own. */ - if (ConstraintNameIsUsed(CONSTRAINT_RELATION, - RelationGetRelid(partition), - fkconstraint->conname)) - conname = ChooseConstraintName(RelationGetRelationName(partition), - ChooseForeignKeyConstraintNameAddition(fkconstraint->fk_attrs), - "fkey", - RelationGetNamespace(partition), NIL); - else - conname = fkconstraint->conname; - constrOid = - CreateConstraintEntry(conname, - RelationGetNamespace(partition), - CONSTRAINT_FOREIGN, - fkconstraint->deferrable, - fkconstraint->initdeferred, - fkconstraint->initially_valid, - parentConstr, - partitionId, - mapped_fkattnum, - numfks, - numfks, - InvalidOid, - indexOid, - RelationGetRelid(pkrel), - pkattnum, - pfeqoperators, - ppeqoperators, - ffeqoperators, - numfks, - fkconstraint->fk_upd_action, - fkconstraint->fk_del_action, - fkdelsetcols, - numfkdelsetcols, - fkconstraint->fk_matchtype, - NULL, - NULL, - NULL, - false, - 1, - false, - false); - - /* - * Give this constraint partition-type dependencies on the parent - * constraint as well as the table. - */ - ObjectAddressSet(address, ConstraintRelationId, constrOid); - ObjectAddressSet(referenced, ConstraintRelationId, parentConstr); - recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_PRI); - ObjectAddressSet(referenced, RelationRelationId, partitionId); - recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_SEC); - - /* Make all this visible before recursing */ - CommandCounterIncrement(); + address = addFkConstraint(addFkReferencingSide, + fkconstraint->conname, fkconstraint, + partition, pkrel, indexOid, parentConstr, + numfks, pkattnum, + mapped_fkattnum, pfeqoperators, + ppeqoperators, ffeqoperators, + numfkdelsetcols, fkdelsetcols, true); /* call ourselves to finalize the creation and we're done */ addFkRecurseReferencing(wqueue, fkconstraint, partition, pkrel, indexOid, - constrOid, + address.objectId, numfks, pkattnum, mapped_fkattnum, @@ -10583,6 +10634,7 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) int numfkdelsetcols; AttrNumber confdelsetcols[INDEX_MAX_KEYS]; Constraint *fkconstraint; + ObjectAddress address; Oid deleteTriggerOid, updateTriggerOid; @@ -10616,7 +10668,7 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) * Because we're only expanding the key space at the referenced side, * we don't need to prevent any operation in the referencing table, so * AccessShareLock suffices (assumes that dropping the constraint - * acquires AEL). + * acquires AccessExclusiveLock). */ fkRel = table_open(constrForm->conrelid, AccessShareLock); @@ -10682,12 +10734,20 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) constrForm->confrelid, constrForm->conrelid, &deleteTriggerOid, &updateTriggerOid); + /* Add this constraint ... */ + address = addFkConstraint(addFkReferencedSide, + fkconstraint->conname, fkconstraint, fkRel, + partitionRel, partIndexId, constrOid, + numfks, mapped_confkey, + conkey, conpfeqop, conppeqop, conffeqop, + numfkdelsetcols, confdelsetcols, false); + /* ... and recurse */ addFkRecurseReferenced(NULL, fkconstraint, fkRel, partitionRel, partIndexId, - constrOid, + address.objectId, numfks, mapped_confkey, conkey, @@ -10717,8 +10777,8 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) * child. * * If wqueue is given, it is used to set up phase-3 verification for each - * cloned constraint; if omitted, we assume that such verification is not - * needed (example: the partition is being created anew). + * cloned constraint; omit it if such verification is not needed + * (example: the partition is being created anew). */ static void CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) @@ -10802,9 +10862,7 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) Constraint *fkconstraint; bool attached; Oid indexOid; - Oid constrOid; - ObjectAddress address, - referenced; + ObjectAddress address; ListCell *lc; Oid insertTriggerOid, updateTriggerOid; @@ -10901,7 +10959,7 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) fkconstraint->old_conpfeqop = NIL; fkconstraint->old_pktable_oid = InvalidOid; fkconstraint->skip_validation = false; - fkconstraint->initially_valid = true; + fkconstraint->initially_valid = constrForm->convalidated; for (int i = 0; i < numfks; i++) { Form_pg_attribute att; @@ -10911,71 +10969,29 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) fkconstraint->fk_attrs = lappend(fkconstraint->fk_attrs, makeString(NameStr(att->attname))); } - if (ConstraintNameIsUsed(CONSTRAINT_RELATION, - RelationGetRelid(partRel), - NameStr(constrForm->conname))) - fkconstraint->conname = - ChooseConstraintName(RelationGetRelationName(partRel), - ChooseForeignKeyConstraintNameAddition(fkconstraint->fk_attrs), - "fkey", - RelationGetNamespace(partRel), NIL); - else - fkconstraint->conname = pstrdup(NameStr(constrForm->conname)); indexOid = constrForm->conindid; - constrOid = - CreateConstraintEntry(fkconstraint->conname, - constrForm->connamespace, - CONSTRAINT_FOREIGN, - fkconstraint->deferrable, - fkconstraint->initdeferred, - constrForm->convalidated, - parentConstrOid, - RelationGetRelid(partRel), - mapped_conkey, - numfks, - numfks, - InvalidOid, /* not a domain constraint */ - indexOid, - constrForm->confrelid, /* same foreign rel */ - confkey, - conpfeqop, - conppeqop, - conffeqop, - numfks, - fkconstraint->fk_upd_action, - fkconstraint->fk_del_action, - confdelsetcols, - numfkdelsetcols, - fkconstraint->fk_matchtype, - NULL, - NULL, - NULL, - false, /* islocal */ - 1, /* inhcount */ - false, /* conNoInherit */ - true); - /* Set up partition dependencies for the new constraint */ - ObjectAddressSet(address, ConstraintRelationId, constrOid); - ObjectAddressSet(referenced, ConstraintRelationId, parentConstrOid); - recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_PRI); - ObjectAddressSet(referenced, RelationRelationId, - RelationGetRelid(partRel)); - recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_SEC); + /* Create the pg_constraint entry at this level */ + address = addFkConstraint(addFkReferencingSide, + NameStr(constrForm->conname), fkconstraint, + partRel, pkrel, indexOid, parentConstrOid, + numfks, confkey, + mapped_conkey, conpfeqop, + conppeqop, conffeqop, + numfkdelsetcols, confdelsetcols, + false); /* Done with the cloned constraint's tuple */ ReleaseSysCache(tuple); - /* Make all this visible before recursing */ - CommandCounterIncrement(); - + /* Create the check triggers, and recurse to partitions, if any */ addFkRecurseReferencing(wqueue, fkconstraint, partRel, pkrel, indexOid, - constrOid, + address.objectId, numfks, confkey, mapped_conkey, @@ -11139,6 +11155,81 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk, TriggerSetParentTrigger(trigrel, updateTriggerOid, parentUpdTrigger, partRelid); + /* + * If the referenced table is partitioned, then the partition we're + * attaching now has extra pg_constraint rows and action triggers that are + * no longer needed. Remove those. + */ + if (get_rel_relkind(fk->confrelid) == RELKIND_PARTITIONED_TABLE) + { + Relation pg_constraint = table_open(ConstraintRelationId, RowShareLock); + ObjectAddresses *objs; + HeapTuple consttup; + + ScanKeyInit(&key, + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(fk->conrelid)); + + scan = systable_beginscan(pg_constraint, + ConstraintRelidTypidNameIndexId, + true, NULL, 1, &key); + objs = new_object_addresses(); + while ((consttup = systable_getnext(scan)) != NULL) + { + Form_pg_constraint conform = (Form_pg_constraint) GETSTRUCT(consttup); + + if (conform->conparentid != fk->conoid) + continue; + else + { + ObjectAddress addr; + SysScanDesc scan2; + ScanKeyData key2; + int n PG_USED_FOR_ASSERTS_ONLY; + + ObjectAddressSet(addr, ConstraintRelationId, conform->oid); + add_exact_object_address(&addr, objs); + + /* + * First we must delete the dependency record that binds the + * constraint records together. + */ + n = deleteDependencyRecordsForSpecific(ConstraintRelationId, + conform->oid, + DEPENDENCY_INTERNAL, + ConstraintRelationId, + fk->conoid); + Assert(n == 1); /* actually only one is expected */ + + /* + * Now search for the triggers for this constraint and set + * them up for deletion too + */ + ScanKeyInit(&key2, + Anum_pg_trigger_tgconstraint, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(conform->oid)); + scan2 = systable_beginscan(trigrel, TriggerConstraintIndexId, + true, NULL, 1, &key2); + while ((trigtup = systable_getnext(scan2)) != NULL) + { + ObjectAddressSet(addr, TriggerRelationId, + ((Form_pg_trigger) GETSTRUCT(trigtup))->oid); + add_exact_object_address(&addr, objs); + } + systable_endscan(scan2); + } + } + /* make the dependency deletions visible */ + CommandCounterIncrement(); + performMultipleDeletions(objs, DROP_RESTRICT, + PERFORM_DELETION_INTERNAL); + systable_endscan(scan); + + table_close(pg_constraint, RowShareLock); + } + CommandCounterIncrement(); return true; } @@ -19132,7 +19223,7 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, } /* Do the final part of detaching */ - DetachPartitionFinalize(rel, partRel, concurrent, defaultPartOid); + DetachPartitionFinalize(wqueue, rel, partRel, concurrent, defaultPartOid); ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel)); @@ -19149,8 +19240,8 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, * transaction of the concurrent algorithm fails (crash or abort). */ static void -DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, - Oid defaultPartOid) +DetachPartitionFinalize(List **wqueue, Relation rel, Relation partRel, + bool concurrent, Oid defaultPartOid) { Relation classRel; List *fks; @@ -19186,8 +19277,11 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, { ForeignKeyCacheInfo *fk = lfirst(cell); HeapTuple contup; + HeapTuple parentConTup; Form_pg_constraint conform; + Form_pg_constraint parentConForm; Constraint *fkconstraint; + Oid parentConstrOid; Oid insertTriggerOid, updateTriggerOid; @@ -19204,7 +19298,20 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, continue; } - /* unset conparentid and adjust conislocal, coninhcount, etc. */ + parentConstrOid = conform->conparentid; + + Assert(OidIsValid(conform->conparentid)); + parentConTup = SearchSysCache1(CONSTROID, + ObjectIdGetDatum(parentConstrOid)); + if (!HeapTupleIsValid(parentConTup)) + elog(ERROR, "cache lookup failed for constraint %u", + conform->conparentid); + parentConForm = (Form_pg_constraint) GETSTRUCT(parentConTup); + + /* + * The constraint on this table must be marked no longer a child of + * the parent's constraint, as do its check triggers. + */ ConstraintSetParentConstraint(fk->conoid, InvalidOid, InvalidOid); /* @@ -19222,35 +19329,88 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, RelationGetRelid(partRel)); /* - * Make the action triggers on the referenced relation. When this was - * a partition the action triggers pointed to the parent rel (they - * still do), but now we need separate ones of our own. + * If the referenced side is partitioned (which we know because our + * parent's constraint points to a different relation than ours) then + * we must, in addition to the above, create pg_constraint rows that + * point to each partition, each with its own action triggers. */ - fkconstraint = makeNode(Constraint); - fkconstraint->contype = CONSTRAINT_FOREIGN; - fkconstraint->conname = pstrdup(NameStr(conform->conname)); - fkconstraint->deferrable = conform->condeferrable; - fkconstraint->initdeferred = conform->condeferred; - fkconstraint->location = -1; - fkconstraint->pktable = NULL; - fkconstraint->fk_attrs = NIL; - fkconstraint->pk_attrs = NIL; - fkconstraint->fk_matchtype = conform->confmatchtype; - fkconstraint->fk_upd_action = conform->confupdtype; - fkconstraint->fk_del_action = conform->confdeltype; - fkconstraint->fk_del_set_cols = NIL; - fkconstraint->old_conpfeqop = NIL; - fkconstraint->old_pktable_oid = InvalidOid; - fkconstraint->skip_validation = false; - fkconstraint->initially_valid = true; + if (parentConForm->conrelid != conform->conrelid) + { + int numfks; + AttrNumber conkey[INDEX_MAX_KEYS]; + AttrMap *attmap; + AttrNumber confkey[INDEX_MAX_KEYS]; + Oid conpfeqop[INDEX_MAX_KEYS]; + Oid conppeqop[INDEX_MAX_KEYS]; + Oid conffeqop[INDEX_MAX_KEYS]; + int numfkdelsetcols; + AttrNumber confdelsetcols[INDEX_MAX_KEYS]; + Relation refdRel; - createForeignKeyActionTriggers(partRel, conform->confrelid, - fkconstraint, fk->conoid, - conform->conindid, - InvalidOid, InvalidOid, - NULL, NULL); + DeconstructFkConstraintRow(contup, + &numfks, + conkey, + confkey, + conpfeqop, + conppeqop, + conffeqop, + &numfkdelsetcols, + confdelsetcols); + + /* Create a synthetic node we'll use throughout */ + fkconstraint = makeNode(Constraint); + fkconstraint->contype = CONSTRAINT_FOREIGN; + fkconstraint->conname = pstrdup(NameStr(conform->conname)); + fkconstraint->deferrable = conform->condeferrable; + fkconstraint->initdeferred = conform->condeferred; + fkconstraint->skip_validation = true; + fkconstraint->initially_valid = true; + /* a few irrelevant fields omitted here */ + fkconstraint->pktable = NULL; + fkconstraint->fk_attrs = NIL; + fkconstraint->pk_attrs = NIL; + fkconstraint->fk_matchtype = conform->confmatchtype; + fkconstraint->fk_upd_action = conform->confupdtype; + fkconstraint->fk_del_action = conform->confdeltype; + fkconstraint->fk_del_set_cols = NIL; + fkconstraint->old_conpfeqop = NIL; + fkconstraint->old_pktable_oid = InvalidOid; + fkconstraint->location = -1; + + attmap = build_attrmap_by_name(RelationGetDescr(partRel), + RelationGetDescr(rel), + false); + for (int i = 0; i < numfks; i++) + { + Form_pg_attribute att; + + att = TupleDescAttr(RelationGetDescr(partRel), + attmap->attnums[conkey[i] - 1] - 1); + fkconstraint->fk_attrs = lappend(fkconstraint->fk_attrs, + makeString(NameStr(att->attname))); + } + + refdRel = table_open(fk->confrelid, AccessShareLock); + + addFkRecurseReferenced(wqueue, fkconstraint, partRel, + refdRel, + conform->conindid, + fk->conoid, + numfks, + confkey, + conkey, + conpfeqop, + conppeqop, + conffeqop, + numfkdelsetcols, + confdelsetcols, + true, + InvalidOid, InvalidOid); + table_close(refdRel, AccessShareLock); + } ReleaseSysCache(contup); + ReleaseSysCache(parentConTup); } list_free_deep(fks); if (trigrel) @@ -19397,7 +19557,7 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, * completion; this completes the detaching process. */ static ObjectAddress -ATExecDetachPartitionFinalize(Relation rel, RangeVar *name) +ATExecDetachPartitionFinalize(List **wqueue, Relation rel, RangeVar *name) { Relation partRel; ObjectAddress address; @@ -19415,7 +19575,7 @@ ATExecDetachPartitionFinalize(Relation rel, RangeVar *name) */ WaitForOlderSnapshots(snap->xmin, false); - DetachPartitionFinalize(rel, partRel, true, InvalidOid); + DetachPartitionFinalize(wqueue, rel, partRel, true, InvalidOid); ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel)); diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 8c04a24b37d..b73e7dced8f 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -2934,3 +2934,99 @@ DETAIL: drop cascades to table fkpart11.pk drop cascades to table fkpart11.fk_parted drop cascades to table fkpart11.fk_another drop cascades to function fkpart11.print_row() +-- When a table is attached as partition to a partitioned table that has +-- a foreign key to another partitioned table, it acquires a clone of the +-- FK. Upon detach, this clone is not removed, but instead becomes an +-- independent FK. If it then attaches to the partitioned table again, +-- the FK from the parent "takes over" ownership of the independent FK rather +-- than creating a separate one. +CREATE SCHEMA fkpart12 + CREATE TABLE fk_p ( id int, jd int, PRIMARY KEY(id, jd)) PARTITION BY list (id) + CREATE TABLE fk_p_1 PARTITION OF fk_p FOR VALUES IN (1) PARTITION BY list (jd) + CREATE TABLE fk_p_1_1 PARTITION OF fk_p_1 FOR VALUES IN (1) + CREATE TABLE fk_p_1_2 PARTITION OF fk_p_1 FOR VALUES IN (2) + CREATE TABLE fk_p_2 PARTITION OF fk_p FOR VALUES IN (2) PARTITION BY list (jd) + CREATE TABLE fk_p_2_1 PARTITION OF fk_p_2 FOR VALUES IN (1) + CREATE TABLE fk_p_2_2 PARTITION OF fk_p_2 FOR VALUES IN (2) + CREATE TABLE fk_r_1 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) + CREATE TABLE fk_r_2 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) PARTITION BY list (id) + CREATE TABLE fk_r_2_1 PARTITION OF fk_r_2 FOR VALUES IN (2, 1) + CREATE TABLE fk_r ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL, + FOREIGN KEY (p_id, p_jd) REFERENCES fk_p (id, jd) + ) PARTITION BY list (id); +SET search_path TO fkpart12; +INSERT INTO fk_p VALUES (1, 1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2); +\d fk_r_2 + Partitioned table "fkpart12.fk_r_2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + id | integer | | not null | + p_id | integer | | not null | + p_jd | integer | | not null | +Partition of: fk_r FOR VALUES IN (2) +Partition key: LIST (id) +Indexes: + "fk_r_2_pkey" PRIMARY KEY, btree (id) +Foreign-key constraints: + TABLE "fk_r" CONSTRAINT "fk_r_p_id_p_jd_fkey" FOREIGN KEY (p_id, p_jd) REFERENCES fk_p(id, jd) +Number of partitions: 1 (Use \d+ to list them.) + +INSERT INTO fk_r VALUES (1, 1, 1); +INSERT INTO fk_r VALUES (2, 2, 1); +ERROR: insert or update on table "fk_r_2_1" violates foreign key constraint "fk_r_p_id_p_jd_fkey" +DETAIL: Key (p_id, p_jd)=(2, 1) is not present in table "fk_p". +ALTER TABLE fk_r DETACH PARTITION fk_r_1; +ALTER TABLE fk_r DETACH PARTITION fk_r_2; +\d fk_r_2 + Partitioned table "fkpart12.fk_r_2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + id | integer | | not null | + p_id | integer | | not null | + p_jd | integer | | not null | +Partition key: LIST (id) +Indexes: + "fk_r_2_pkey" PRIMARY KEY, btree (id) +Foreign-key constraints: + "fk_r_p_id_p_jd_fkey" FOREIGN KEY (p_id, p_jd) REFERENCES fk_p(id, jd) +Number of partitions: 1 (Use \d+ to list them.) + +INSERT INTO fk_r_1 VALUES (2, 1, 2); -- should fail +ERROR: insert or update on table "fk_r_1" violates foreign key constraint "fk_r_p_id_p_jd_fkey" +DETAIL: Key (p_id, p_jd)=(1, 2) is not present in table "fk_p". +DELETE FROM fk_p; -- should fail +ERROR: update or delete on table "fk_p_1_1" violates foreign key constraint "fk_r_1_p_id_p_jd_fkey1" on table "fk_r_1" +DETAIL: Key (id, jd)=(1, 1) is still referenced from table "fk_r_1". +ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2); +\d fk_r_2 + Partitioned table "fkpart12.fk_r_2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + id | integer | | not null | + p_id | integer | | not null | + p_jd | integer | | not null | +Partition of: fk_r FOR VALUES IN (2) +Partition key: LIST (id) +Indexes: + "fk_r_2_pkey" PRIMARY KEY, btree (id) +Foreign-key constraints: + TABLE "fk_r" CONSTRAINT "fk_r_p_id_p_jd_fkey" FOREIGN KEY (p_id, p_jd) REFERENCES fk_p(id, jd) +Number of partitions: 1 (Use \d+ to list them.) + +DELETE FROM fk_p; -- should fail +ERROR: update or delete on table "fk_p_1_1" violates foreign key constraint "fk_r_p_id_p_jd_fkey2" on table "fk_r" +DETAIL: Key (id, jd)=(1, 1) is still referenced from table "fk_r". +-- these should all fail +ALTER TABLE fk_r_1 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; +ERROR: cannot drop inherited constraint "fk_r_p_id_p_jd_fkey" of relation "fk_r_1" +ALTER TABLE fk_r DROP CONSTRAINT fk_r_p_id_p_jd_fkey1; +ERROR: cannot drop inherited constraint "fk_r_p_id_p_jd_fkey1" of relation "fk_r" +ALTER TABLE fk_r_2 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; +ERROR: cannot drop inherited constraint "fk_r_p_id_p_jd_fkey" of relation "fk_r_2" +SET client_min_messages TO warning; +DROP SCHEMA fkpart12 CASCADE; +RESET client_min_messages; +RESET search_path; diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index d1aac5357f0..9b2a6b6bff7 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -2086,3 +2086,60 @@ UPDATE fkpart11.pk SET a = 3 WHERE a = 4; UPDATE fkpart11.pk SET a = 1 WHERE a = 2; DROP SCHEMA fkpart11 CASCADE; + +-- When a table is attached as partition to a partitioned table that has +-- a foreign key to another partitioned table, it acquires a clone of the +-- FK. Upon detach, this clone is not removed, but instead becomes an +-- independent FK. If it then attaches to the partitioned table again, +-- the FK from the parent "takes over" ownership of the independent FK rather +-- than creating a separate one. +CREATE SCHEMA fkpart12 + CREATE TABLE fk_p ( id int, jd int, PRIMARY KEY(id, jd)) PARTITION BY list (id) + CREATE TABLE fk_p_1 PARTITION OF fk_p FOR VALUES IN (1) PARTITION BY list (jd) + CREATE TABLE fk_p_1_1 PARTITION OF fk_p_1 FOR VALUES IN (1) + CREATE TABLE fk_p_1_2 PARTITION OF fk_p_1 FOR VALUES IN (2) + CREATE TABLE fk_p_2 PARTITION OF fk_p FOR VALUES IN (2) PARTITION BY list (jd) + CREATE TABLE fk_p_2_1 PARTITION OF fk_p_2 FOR VALUES IN (1) + CREATE TABLE fk_p_2_2 PARTITION OF fk_p_2 FOR VALUES IN (2) + CREATE TABLE fk_r_1 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) + CREATE TABLE fk_r_2 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) PARTITION BY list (id) + CREATE TABLE fk_r_2_1 PARTITION OF fk_r_2 FOR VALUES IN (2, 1) + CREATE TABLE fk_r ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL, + FOREIGN KEY (p_id, p_jd) REFERENCES fk_p (id, jd) + ) PARTITION BY list (id); +SET search_path TO fkpart12; + +INSERT INTO fk_p VALUES (1, 1); + +ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2); + +\d fk_r_2 + +INSERT INTO fk_r VALUES (1, 1, 1); +INSERT INTO fk_r VALUES (2, 2, 1); + +ALTER TABLE fk_r DETACH PARTITION fk_r_1; +ALTER TABLE fk_r DETACH PARTITION fk_r_2; + +\d fk_r_2 + +INSERT INTO fk_r_1 VALUES (2, 1, 2); -- should fail +DELETE FROM fk_p; -- should fail + +ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2); + +\d fk_r_2 + +DELETE FROM fk_p; -- should fail + +-- these should all fail +ALTER TABLE fk_r_1 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; +ALTER TABLE fk_r DROP CONSTRAINT fk_r_p_id_p_jd_fkey1; +ALTER TABLE fk_r_2 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; + +SET client_min_messages TO warning; +DROP SCHEMA fkpart12 CASCADE; +RESET client_min_messages; +RESET search_path; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index d4e9515e9f4..5628b3f09f4 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3250,6 +3250,7 @@ _stringlist access_vector_t acquireLocksOnSubLinks_context add_nulling_relids_context +addFkConstraintSides adjust_appendrel_attrs_context allocfunc amadjustmembers_function -- 2.39.5 --mr6adwpq5l3ngngx Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="v4_master-0001-Redo-foreign-key-maintenance-during-partit.patch" From ae828d23c6158f31d41c0fdba215807f14cada38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= Date: Fri, 18 Oct 2024 16:38:34 +0200 Subject: [PATCH v4_master] Redo foreign key maintenance during partition ATTACH/DETACH ... when the reference table is partitioned. It turns out that the catalog representation we chose for foreign keys connecting partitioned tables is suboptimal, particularly in the sense that a standalone table has a different way to represent it when referencing a partitioned table, than when the same table becomes a partition (and vice versa). This difference means we need to create additional catalog rows on detach (pg_constraint and pg_trigger), and remove them on attach. As a very obvious symptom, we were missing action triggers after detach, which means that you could update/delete rows from the referenced partitioned table that still had referencing rows at the other side previously detached, and fail to throw the required errors. !!!!!!!!! Note that this means existing FKs that refer to partitioned tables might have rows that break relational integrity, on tables that were once partitions. Another possible problem is that trying to reattach a table that had been detached would fail indicating that internal triggers cannot be found, which from the user's point of view is nonsensical. In branches 15 and above, we fix this by creating a new helper function addFkConstraint() which is in charge of creating a standalone pg_constraint row, and repurposing addFkRecurseReferencing() and addFkRecurseReferenced() so that they're only the recursive routine for each side of the FK, and they call addFkConstraint() to create pg_constraint at each partitioning level and add the necessary triggers. These new routines can be used during partition creation, partition attach and detach, and foreign key creation. This reduces redundant code and simplifies the flow. In branches 14 and 13, we have a much simpler fix that consists on simply removing the constraint on detach. The reason is that those branches are missing commit f4566345cf40, which reworked the way this works in a way that we didn't consider back-patchable at the time. I doubt we want to change that now, so this is probably the best we can do there. We opted to leave branch 12 alone, because it's going to end maintenance very soon, and any rushed patch might be worse. In the future we might want to rethink the catalog representation to avoid this problem, but for now the code seems to do what's required to make the constraints operate correctly. Co-authored-by: Jehan-Guillaume de Rorthais Co-authored-by: Tender Wang Co-authored-by: Alvaro Herrera Reported-by: Guillaume Lelarge Reported-by: Jehan-Guillaume de Rorthais Reported-by: Thomas Baehler (SBB CFF FFS) Discussion: https://postgr.es/m/20230420144344.40744130@karst Discussion: https://postgr.es/m/20230705233028.2f554f73@karst Discussion: https://postgr.es/m/GVAP278MB02787E7134FD691861635A8BC9032@GVAP278MB0278.CHEP278.PROD.OUTLOOK.COM Discussion: https://postgr.es/m/18541-628a61bc267cd2d3@postgresql.org --- src/backend/commands/tablecmds.c | 644 ++++++++++++++-------- src/test/regress/expected/foreign_key.out | 96 ++++ src/test/regress/sql/foreign_key.sql | 57 ++ src/tools/pgindent/typedefs.list | 1 + 4 files changed, 557 insertions(+), 241 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 1ccc80087c3..1fb701fbc55 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -349,6 +349,14 @@ typedef struct ForeignTruncateInfo List *rels; } ForeignTruncateInfo; +/* Partial or complete FK creation in addFkConstraint() */ +typedef enum addFkConstraintSides +{ + addFkReferencedSide, + addFkReferencingSide, + addFkBothSides, +} addFkConstraintSides; + /* * Partition tables are expected to be dropped when the parent partitioned * table gets dropped. Hence for partitioning we use AUTO dependency. @@ -508,17 +516,27 @@ static ObjectAddress ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo * Relation rel, Constraint *fkconstraint, bool recurse, bool recursing, LOCKMODE lockmode); -static ObjectAddress addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, - Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr, - int numfks, int16 *pkattnum, int16 *fkattnum, - Oid *pfeqoperators, Oid *ppeqoperators, Oid *ffeqoperators, - int numfkdelsetcols, int16 *fkdelsetcols, - bool old_check_ok, - Oid parentDelTrigger, Oid parentUpdTrigger, - bool with_period); static void validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums, int numfksetcols, const int16 *fksetcolsattnums, List *fksetcols); +static ObjectAddress addFkConstraint(addFkConstraintSides fkside, + char *constraintname, + Constraint *fkconstraint, Relation rel, + Relation pkrel, Oid indexOid, + Oid parentConstr, + int numfks, int16 *pkattnum, int16 *fkattnum, + Oid *pfeqoperators, Oid *ppeqoperators, + Oid *ffeqoperators, int numfkdelsetcols, + int16 *fkdelsetcols, bool is_internal, + bool with_period); +static void addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, + Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr, + int numfks, int16 *pkattnum, int16 *fkattnum, + Oid *pfeqoperators, Oid *ppeqoperators, Oid *ffeqoperators, + int numfkdelsetcols, int16 *fkdelsetcols, + bool old_check_ok, + Oid parentDelTrigger, Oid parentUpdTrigger, + bool with_period); static void addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr, int numfks, int16 *pkattnum, int16 *fkattnum, @@ -527,7 +545,6 @@ static void addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, bool old_check_ok, LOCKMODE lockmode, Oid parentInsTrigger, Oid parentUpdTrigger, bool with_period); - static void CloneForeignKeyConstraints(List **wqueue, Relation parentRel, Relation partitionRel); static void CloneFkReferenced(Relation parentRel, Relation partitionRel); @@ -652,9 +669,11 @@ static void DropClonedTriggersFromPartition(Oid partitionId); static ObjectAddress ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, RangeVar *name, bool concurrent); -static void DetachPartitionFinalize(Relation rel, Relation partRel, - bool concurrent, Oid defaultPartOid); -static ObjectAddress ATExecDetachPartitionFinalize(Relation rel, RangeVar *name); +static void DetachPartitionFinalize(List **wqueue, Relation rel, + Relation partRel, bool concurrent, + Oid defaultPartOid); +static ObjectAddress ATExecDetachPartitionFinalize(List **wqueue, Relation rel, + RangeVar *name); static ObjectAddress ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name); static void validatePartitionedIndex(Relation partedIdx, Relation partedTbl); @@ -5525,7 +5544,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, ((PartitionCmd *) cmd->def)->concurrent); break; case AT_DetachPartitionFinalize: - address = ATExecDetachPartitionFinalize(rel, ((PartitionCmd *) cmd->def)->name); + address = ATExecDetachPartitionFinalize(wqueue, rel, ((PartitionCmd *) cmd->def)->name); break; default: /* oops */ elog(ERROR, "unrecognized alter table type: %d", @@ -10045,26 +10064,39 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, FindFKPeriodOpers(opclasses[numpks - 1], &periodoperoid, &aggedperiodoperoid); } - /* - * Create all the constraint and trigger objects, recursing to partitions - * as necessary. First handle the referenced side. - */ - address = addFkRecurseReferenced(wqueue, fkconstraint, rel, pkrel, - indexOid, - InvalidOid, /* no parent constraint */ - numfks, - pkattnum, - fkattnum, - pfeqoperators, - ppeqoperators, - ffeqoperators, - numfkdelsetcols, - fkdelsetcols, - old_check_ok, - InvalidOid, InvalidOid, - with_period); + /* First, create the constraint catalog entry itself. */ + address = addFkConstraint(addFkBothSides, + fkconstraint->conname, fkconstraint, rel, pkrel, + indexOid, + InvalidOid, /* no parent constraint */ + numfks, + pkattnum, + fkattnum, + pfeqoperators, + ppeqoperators, + ffeqoperators, + numfkdelsetcols, + fkdelsetcols, + false, + with_period); - /* Now handle the referencing side. */ + /* Next process the action triggers at the referenced side and recurse */ + addFkRecurseReferenced(wqueue, fkconstraint, rel, pkrel, + indexOid, + address.objectId, + numfks, + pkattnum, + fkattnum, + pfeqoperators, + ppeqoperators, + ffeqoperators, + numfkdelsetcols, + fkdelsetcols, + old_check_ok, + InvalidOid, InvalidOid, + with_period); + + /* Lastly create the check triggers at the referencing side and recurse */ addFkRecurseReferencing(wqueue, fkconstraint, rel, pkrel, indexOid, address.objectId, @@ -10125,47 +10157,42 @@ validateFkOnDeleteSetColumns(int numfks, const int16 *fkattnums, } /* - * addFkRecurseReferenced - * subroutine for ATAddForeignKeyConstraint; recurses on the referenced - * side of the constraint + * addFkConstraint + * Install pg_constraint entries to implement a foreign key constraint. + * Caller must separately invoke addFkRecurseReferenced and + * addFkRecurseReferencing, as appropriate, to install pg_trigger entries + * and (for partitioned tables) recurse to partitions. * - * Create pg_constraint rows for the referenced side of the constraint, - * referencing the parent of the referencing side; also create action triggers - * on leaf partitions. If the table is partitioned, recurse to handle each - * partition. - * - * wqueue is the ALTER TABLE work queue; can be NULL when not running as part - * of an ALTER TABLE sequence. - * fkconstraint is the constraint being added. - * rel is the root referencing relation. - * pkrel is the referenced relation; might be a partition, if recursing. - * indexOid is the OID of the index (on pkrel) implementing this constraint. - * parentConstr is the OID of a parent constraint; InvalidOid if this is a - * top-level constraint. - * numfks is the number of columns in the foreign key - * pkattnum is the attnum array of referenced attributes. - * fkattnum is the attnum array of referencing attributes. - * numfkdelsetcols is the number of columns in the ON DELETE SET NULL/DEFAULT - * (...) clause - * fkdelsetcols is the attnum array of the columns in the ON DELETE SET - * NULL/DEFAULT clause - * pf/pp/ffeqoperators are OID array of operators between columns. - * old_check_ok signals that this constraint replaces an existing one that - * was already validated (thus this one doesn't need validation). - * parentDelTrigger and parentUpdTrigger, when being recursively called on - * a partition, are the OIDs of the parent action triggers for DELETE and - * UPDATE respectively. + * * fkside indicates which side of the FK (or both) to create. Caller must + * call addFkRecurseReferenced if it's addFkReferencedSide, + * addFkRecurseReferencing if it's addFkReferencingSide, or both if it's + * addFkBothSides. + * * constraintname the base name for the constraint being added. This is + * copied to fkconstraint->conname if the latter is not set. + * * fkconstraint is the constraint being added. + * * rel is the root referencing relation. + * * pkrel is the referenced relation; might be a partition, if recursing. + * * indexOid is the OID of the index (on pkrel) implementing this constraint. + * * parentConstr is the OID of a parent constraint; InvalidOid if this is a + * top-level constraint. + * * numfks is the number of columns in the foreign key. + * * pkattnum is the attnum array of referenced attributes. + * * fkattnum is the attnum array of referencing attributes. + * * pf/pp/ffeqoperators are OID array of operators between columns. + * * numfkdelsetcols is the number of columns in the ON DELETE SET NULL/DEFAULT + * (...) clause. + * * fkdelsetcols is the attnum array of the columns in the ON DELETE SET + * NULL/DEFAULT clause. + * - with_period: true if this is a temporal FKs */ static ObjectAddress -addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, - Relation pkrel, Oid indexOid, Oid parentConstr, - int numfks, - int16 *pkattnum, int16 *fkattnum, Oid *pfeqoperators, - Oid *ppeqoperators, Oid *ffeqoperators, - int numfkdelsetcols, int16 *fkdelsetcols, - bool old_check_ok, - Oid parentDelTrigger, Oid parentUpdTrigger, - bool with_period) +addFkConstraint(addFkConstraintSides fkside, + char *constraintname, Constraint *fkconstraint, + Relation rel, Relation pkrel, Oid indexOid, Oid parentConstr, + int numfks, int16 *pkattnum, + int16 *fkattnum, Oid *pfeqoperators, Oid *ppeqoperators, + Oid *ffeqoperators, int numfkdelsetcols, int16 *fkdelsetcols, + bool is_internal, bool with_period) { ObjectAddress address; Oid constrOid; @@ -10173,8 +10200,6 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, bool conislocal; int16 coninhcount; bool connoinherit; - Oid deleteTriggerOid, - updateTriggerOid; /* * Verify relkind for each referenced partition. At the top level, this @@ -10193,13 +10218,16 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, */ if (ConstraintNameIsUsed(CONSTRAINT_RELATION, RelationGetRelid(rel), - fkconstraint->conname)) + constraintname)) conname = ChooseConstraintName(RelationGetRelationName(rel), ChooseForeignKeyConstraintNameAddition(fkconstraint->fk_attrs), "fkey", RelationGetNamespace(rel), NIL); else - conname = fkconstraint->conname; + conname = constraintname; + + if (fkconstraint->conname == NULL) + fkconstraint->conname = pstrdup(conname); if (OidIsValid(parentConstr)) { @@ -10252,33 +10280,97 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, coninhcount, /* inhcount */ connoinherit, /* conNoInherit */ with_period, /* conPeriod */ - false); /* is_internal */ + is_internal); /* is_internal */ ObjectAddressSet(address, ConstraintRelationId, constrOid); - /* - * Mark the child constraint as part of the parent constraint; it must not - * be dropped on its own. (This constraint is deleted when the partition - * is detached, but a special check needs to occur that the partition - * contains no referenced values.) - */ if (OidIsValid(parentConstr)) { ObjectAddress referenced; ObjectAddressSet(referenced, ConstraintRelationId, parentConstr); - recordDependencyOn(&address, &referenced, DEPENDENCY_INTERNAL); + if (fkside == addFkReferencedSide) + { + /* + * Mark the child constraint as part of the parent constraint; it + * must not be dropped on its own. (This constraint is deleted + * when the partition is detached, but a special check needs to + * occur that the partition contains no referenced values.) + */ + recordDependencyOn(&address, &referenced, DEPENDENCY_INTERNAL); + } + else + { + /* + * Give this constraint partition-type dependencies on the parent + * constraint as well as the table. + */ + recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_PRI); + ObjectAddressSet(referenced, RelationRelationId, RelationGetRelid(rel)); + recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_SEC); + } } /* make new constraint visible, in case we add more */ CommandCounterIncrement(); + return address; +} + +/* + * addFkRecurseReferenced + * Recursive helper for the referenced side of foreign key creation, + * which creates the action triggers and recurses + * + * If the referenced relation is a plain relation, create the necessary action + * triggers that implement the constraint. If the referenced relation is a + * partitioned table, then we create a pg_constraint row referencing the parent + * of the referencing side for it and recurse on this routine for each + * partition. + * + * wqueue is the ALTER TABLE work queue; can be NULL when not running as part + * of an ALTER TABLE sequence. + * fkconstraint is the constraint being added. + * rel is the root referencing relation. + * pkrel is the referenced relation; might be a partition, if recursing. + * indexOid is the OID of the index (on pkrel) implementing this constraint. + * parentConstr is the OID of a parent constraint; InvalidOid if this is a + * top-level constraint. + * numfks is the number of columns in the foreign key + * pkattnum is the attnum array of referenced attributes. + * fkattnum is the attnum array of referencing attributes. + * numfkdelsetcols is the number of columns in the ON DELETE SET NULL/DEFAULT + * (...) clause + * fkdelsetcols is the attnum array of the columns in the ON DELETE SET + * NULL/DEFAULT clause + * pf/pp/ffeqoperators are OID array of operators between columns. + * old_check_ok signals that this constraint replaces an existing one that + * was already validated (thus this one doesn't need validation). + * parentDelTrigger and parentUpdTrigger, when being recursively called on + * a partition, are the OIDs of the parent action triggers for DELETE and + * UPDATE respectively. + * with_period: true if this is a temporal FKs + */ +static void +addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, + Relation pkrel, Oid indexOid, Oid parentConstr, + int numfks, + int16 *pkattnum, int16 *fkattnum, Oid *pfeqoperators, + Oid *ppeqoperators, Oid *ffeqoperators, + int numfkdelsetcols, int16 *fkdelsetcols, + bool old_check_ok, + Oid parentDelTrigger, Oid parentUpdTrigger, + bool with_period) +{ + Oid deleteTriggerOid, + updateTriggerOid; + /* * Create the action triggers that enforce the constraint. */ createForeignKeyActionTriggers(rel, RelationGetRelid(pkrel), fkconstraint, - constrOid, indexOid, + parentConstr, indexOid, parentDelTrigger, parentUpdTrigger, &deleteTriggerOid, &updateTriggerOid); @@ -10297,6 +10389,7 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, AttrMap *map; AttrNumber *mapped_pkattnum; Oid partIndexId; + ObjectAddress address; partRel = table_open(pd->oids[i], ShareRowExclusiveLock); @@ -10316,13 +10409,23 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, else mapped_pkattnum = pkattnum; - /* do the deed */ + /* Determine the index to use at this level */ partIndexId = index_get_partition(partRel, indexOid); if (!OidIsValid(partIndexId)) elog(ERROR, "index for %u not found in partition %s", indexOid, RelationGetRelationName(partRel)); + + /* Create entry at this level ... */ + address = addFkConstraint(addFkReferencedSide, + fkconstraint->conname, fkconstraint, rel, + partRel, partIndexId, parentConstr, + numfks, mapped_pkattnum, + fkattnum, pfeqoperators, ppeqoperators, + ffeqoperators, numfkdelsetcols, + fkdelsetcols, true, with_period); + /* ... and recurse to our children */ addFkRecurseReferenced(wqueue, fkconstraint, rel, partRel, - partIndexId, constrOid, numfks, + partIndexId, address.objectId, numfks, mapped_pkattnum, fkattnum, pfeqoperators, ppeqoperators, ffeqoperators, numfkdelsetcols, fkdelsetcols, @@ -10339,13 +10442,12 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, } } } - - return address; } /* * addFkRecurseReferencing - * subroutine for ATAddForeignKeyConstraint and CloneFkReferencing + * Recursive helper for the referencing side of foreign key creation, + * which creates the check triggers and recurses * * If the referencing relation is a plain relation, create the necessary check * triggers that implement the constraint, and set up for Phase 3 constraint @@ -10467,10 +10569,7 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, AttrMap *attmap; AttrNumber mapped_fkattnum[INDEX_MAX_KEYS]; bool attached; - char *conname; - Oid constrOid; - ObjectAddress address, - referenced; + ObjectAddress address; ListCell *cell; CheckAlterTableIsSafe(partition); @@ -10513,66 +10612,19 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, /* * No luck finding a good constraint to reuse; create our own. */ - if (ConstraintNameIsUsed(CONSTRAINT_RELATION, - RelationGetRelid(partition), - fkconstraint->conname)) - conname = ChooseConstraintName(RelationGetRelationName(partition), - ChooseForeignKeyConstraintNameAddition(fkconstraint->fk_attrs), - "fkey", - RelationGetNamespace(partition), NIL); - else - conname = fkconstraint->conname; - constrOid = - CreateConstraintEntry(conname, - RelationGetNamespace(partition), - CONSTRAINT_FOREIGN, - fkconstraint->deferrable, - fkconstraint->initdeferred, - fkconstraint->initially_valid, - parentConstr, - partitionId, - mapped_fkattnum, - numfks, - numfks, - InvalidOid, - indexOid, - RelationGetRelid(pkrel), - pkattnum, - pfeqoperators, - ppeqoperators, - ffeqoperators, - numfks, - fkconstraint->fk_upd_action, - fkconstraint->fk_del_action, - fkdelsetcols, - numfkdelsetcols, - fkconstraint->fk_matchtype, - NULL, - NULL, - NULL, - false, /* conIsLocal */ - 1, /* conInhCount */ - false, /* conNoInherit */ - with_period, /* conPeriod */ - false); - - /* - * Give this constraint partition-type dependencies on the parent - * constraint as well as the table. - */ - ObjectAddressSet(address, ConstraintRelationId, constrOid); - ObjectAddressSet(referenced, ConstraintRelationId, parentConstr); - recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_PRI); - ObjectAddressSet(referenced, RelationRelationId, partitionId); - recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_SEC); - - /* Make all this visible before recursing */ - CommandCounterIncrement(); + address = addFkConstraint(addFkReferencingSide, + fkconstraint->conname, fkconstraint, + partition, pkrel, indexOid, parentConstr, + numfks, pkattnum, + mapped_fkattnum, pfeqoperators, + ppeqoperators, ffeqoperators, + numfkdelsetcols, fkdelsetcols, true, + with_period); /* call ourselves to finalize the creation and we're done */ addFkRecurseReferencing(wqueue, fkconstraint, partition, pkrel, indexOid, - constrOid, + address.objectId, numfks, pkattnum, mapped_fkattnum, @@ -10706,6 +10758,7 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) int numfkdelsetcols; AttrNumber confdelsetcols[INDEX_MAX_KEYS]; Constraint *fkconstraint; + ObjectAddress address; Oid deleteTriggerOid, updateTriggerOid; @@ -10739,7 +10792,7 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) * Because we're only expanding the key space at the referenced side, * we don't need to prevent any operation in the referencing table, so * AccessShareLock suffices (assumes that dropping the constraint - * acquires AEL). + * acquires AccessExclusiveLock). */ fkRel = table_open(constrForm->conrelid, AccessShareLock); @@ -10805,12 +10858,21 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) constrForm->confrelid, constrForm->conrelid, &deleteTriggerOid, &updateTriggerOid); + /* Add this constraint ... */ + address = addFkConstraint(addFkReferencedSide, + fkconstraint->conname, fkconstraint, fkRel, + partitionRel, partIndexId, constrOid, + numfks, mapped_confkey, + conkey, conpfeqop, conppeqop, conffeqop, + numfkdelsetcols, confdelsetcols, false, + constrForm->conperiod); + /* ... and recurse */ addFkRecurseReferenced(NULL, fkconstraint, fkRel, partitionRel, partIndexId, - constrOid, + address.objectId, numfks, mapped_confkey, conkey, @@ -10841,8 +10903,8 @@ CloneFkReferenced(Relation parentRel, Relation partitionRel) * child. * * If wqueue is given, it is used to set up phase-3 verification for each - * cloned constraint; if omitted, we assume that such verification is not - * needed (example: the partition is being created anew). + * cloned constraint; omit it if such verification is not needed + * (example: the partition is being created anew). */ static void CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) @@ -10926,9 +10988,7 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) Constraint *fkconstraint; bool attached; Oid indexOid; - Oid constrOid; - ObjectAddress address, - referenced; + ObjectAddress address; ListCell *lc; Oid insertTriggerOid, updateTriggerOid; @@ -11026,7 +11086,7 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) fkconstraint->old_conpfeqop = NIL; fkconstraint->old_pktable_oid = InvalidOid; fkconstraint->skip_validation = false; - fkconstraint->initially_valid = true; + fkconstraint->initially_valid = constrForm->convalidated; for (int i = 0; i < numfks; i++) { Form_pg_attribute att; @@ -11036,73 +11096,30 @@ CloneFkReferencing(List **wqueue, Relation parentRel, Relation partRel) fkconstraint->fk_attrs = lappend(fkconstraint->fk_attrs, makeString(NameStr(att->attname))); } - if (ConstraintNameIsUsed(CONSTRAINT_RELATION, - RelationGetRelid(partRel), - NameStr(constrForm->conname))) - fkconstraint->conname = - ChooseConstraintName(RelationGetRelationName(partRel), - ChooseForeignKeyConstraintNameAddition(fkconstraint->fk_attrs), - "fkey", - RelationGetNamespace(partRel), NIL); - else - fkconstraint->conname = pstrdup(NameStr(constrForm->conname)); indexOid = constrForm->conindid; with_period = constrForm->conperiod; - constrOid = - CreateConstraintEntry(fkconstraint->conname, - constrForm->connamespace, - CONSTRAINT_FOREIGN, - fkconstraint->deferrable, - fkconstraint->initdeferred, - constrForm->convalidated, - parentConstrOid, - RelationGetRelid(partRel), - mapped_conkey, - numfks, - numfks, - InvalidOid, /* not a domain constraint */ - indexOid, - constrForm->confrelid, /* same foreign rel */ - confkey, - conpfeqop, - conppeqop, - conffeqop, - numfks, - fkconstraint->fk_upd_action, - fkconstraint->fk_del_action, - confdelsetcols, - numfkdelsetcols, - fkconstraint->fk_matchtype, - NULL, - NULL, - NULL, - false, /* conIsLocal */ - 1, /* conInhCount */ - false, /* conNoInherit */ - with_period, /* conPeriod */ - true); - /* Set up partition dependencies for the new constraint */ - ObjectAddressSet(address, ConstraintRelationId, constrOid); - ObjectAddressSet(referenced, ConstraintRelationId, parentConstrOid); - recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_PRI); - ObjectAddressSet(referenced, RelationRelationId, - RelationGetRelid(partRel)); - recordDependencyOn(&address, &referenced, DEPENDENCY_PARTITION_SEC); + /* Create the pg_constraint entry at this level */ + address = addFkConstraint(addFkReferencingSide, + NameStr(constrForm->conname), fkconstraint, + partRel, pkrel, indexOid, parentConstrOid, + numfks, confkey, + mapped_conkey, conpfeqop, + conppeqop, conffeqop, + numfkdelsetcols, confdelsetcols, + false, with_period); /* Done with the cloned constraint's tuple */ ReleaseSysCache(tuple); - /* Make all this visible before recursing */ - CommandCounterIncrement(); - + /* Create the check triggers, and recurse to partitions, if any */ addFkRecurseReferencing(wqueue, fkconstraint, partRel, pkrel, indexOid, - constrOid, + address.objectId, numfks, confkey, mapped_conkey, @@ -11267,6 +11284,81 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk, TriggerSetParentTrigger(trigrel, updateTriggerOid, parentUpdTrigger, partRelid); + /* + * If the referenced table is partitioned, then the partition we're + * attaching now has extra pg_constraint rows and action triggers that are + * no longer needed. Remove those. + */ + if (get_rel_relkind(fk->confrelid) == RELKIND_PARTITIONED_TABLE) + { + Relation pg_constraint = table_open(ConstraintRelationId, RowShareLock); + ObjectAddresses *objs; + HeapTuple consttup; + + ScanKeyInit(&key, + Anum_pg_constraint_conrelid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(fk->conrelid)); + + scan = systable_beginscan(pg_constraint, + ConstraintRelidTypidNameIndexId, + true, NULL, 1, &key); + objs = new_object_addresses(); + while ((consttup = systable_getnext(scan)) != NULL) + { + Form_pg_constraint conform = (Form_pg_constraint) GETSTRUCT(consttup); + + if (conform->conparentid != fk->conoid) + continue; + else + { + ObjectAddress addr; + SysScanDesc scan2; + ScanKeyData key2; + int n PG_USED_FOR_ASSERTS_ONLY; + + ObjectAddressSet(addr, ConstraintRelationId, conform->oid); + add_exact_object_address(&addr, objs); + + /* + * First we must delete the dependency record that binds the + * constraint records together. + */ + n = deleteDependencyRecordsForSpecific(ConstraintRelationId, + conform->oid, + DEPENDENCY_INTERNAL, + ConstraintRelationId, + fk->conoid); + Assert(n == 1); /* actually only one is expected */ + + /* + * Now search for the triggers for this constraint and set + * them up for deletion too + */ + ScanKeyInit(&key2, + Anum_pg_trigger_tgconstraint, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(conform->oid)); + scan2 = systable_beginscan(trigrel, TriggerConstraintIndexId, + true, NULL, 1, &key2); + while ((trigtup = systable_getnext(scan2)) != NULL) + { + ObjectAddressSet(addr, TriggerRelationId, + ((Form_pg_trigger) GETSTRUCT(trigtup))->oid); + add_exact_object_address(&addr, objs); + } + systable_endscan(scan2); + } + } + /* make the dependency deletions visible */ + CommandCounterIncrement(); + performMultipleDeletions(objs, DROP_RESTRICT, + PERFORM_DELETION_INTERNAL); + systable_endscan(scan); + + table_close(pg_constraint, RowShareLock); + } + CommandCounterIncrement(); return true; } @@ -19284,7 +19376,7 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, } /* Do the final part of detaching */ - DetachPartitionFinalize(rel, partRel, concurrent, defaultPartOid); + DetachPartitionFinalize(wqueue, rel, partRel, concurrent, defaultPartOid); ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel)); @@ -19301,8 +19393,8 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, * transaction of the concurrent algorithm fails (crash or abort). */ static void -DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, - Oid defaultPartOid) +DetachPartitionFinalize(List **wqueue, Relation rel, Relation partRel, + bool concurrent, Oid defaultPartOid) { Relation classRel; List *fks; @@ -19338,8 +19430,11 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, { ForeignKeyCacheInfo *fk = lfirst(cell); HeapTuple contup; + HeapTuple parentConTup; Form_pg_constraint conform; + Form_pg_constraint parentConForm; Constraint *fkconstraint; + Oid parentConstrOid; Oid insertTriggerOid, updateTriggerOid; @@ -19356,7 +19451,20 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, continue; } - /* unset conparentid and adjust conislocal, coninhcount, etc. */ + parentConstrOid = conform->conparentid; + + Assert(OidIsValid(conform->conparentid)); + parentConTup = SearchSysCache1(CONSTROID, + ObjectIdGetDatum(parentConstrOid)); + if (!HeapTupleIsValid(parentConTup)) + elog(ERROR, "cache lookup failed for constraint %u", + conform->conparentid); + parentConForm = (Form_pg_constraint) GETSTRUCT(parentConTup); + + /* + * The constraint on this table must be marked no longer a child of + * the parent's constraint, as do its check triggers. + */ ConstraintSetParentConstraint(fk->conoid, InvalidOid, InvalidOid); /* @@ -19374,35 +19482,89 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, RelationGetRelid(partRel)); /* - * Make the action triggers on the referenced relation. When this was - * a partition the action triggers pointed to the parent rel (they - * still do), but now we need separate ones of our own. + * If the referenced side is partitioned (which we know because our + * parent's constraint points to a different relation than ours) then + * we must, in addition to the above, create pg_constraint rows that + * point to each partition, each with its own action triggers. */ - fkconstraint = makeNode(Constraint); - fkconstraint->contype = CONSTRAINT_FOREIGN; - fkconstraint->conname = pstrdup(NameStr(conform->conname)); - fkconstraint->deferrable = conform->condeferrable; - fkconstraint->initdeferred = conform->condeferred; - fkconstraint->location = -1; - fkconstraint->pktable = NULL; - fkconstraint->fk_attrs = NIL; - fkconstraint->pk_attrs = NIL; - fkconstraint->fk_matchtype = conform->confmatchtype; - fkconstraint->fk_upd_action = conform->confupdtype; - fkconstraint->fk_del_action = conform->confdeltype; - fkconstraint->fk_del_set_cols = NIL; - fkconstraint->old_conpfeqop = NIL; - fkconstraint->old_pktable_oid = InvalidOid; - fkconstraint->skip_validation = false; - fkconstraint->initially_valid = true; + if (parentConForm->conrelid != conform->conrelid) + { + int numfks; + AttrNumber conkey[INDEX_MAX_KEYS]; + AttrMap *attmap; + AttrNumber confkey[INDEX_MAX_KEYS]; + Oid conpfeqop[INDEX_MAX_KEYS]; + Oid conppeqop[INDEX_MAX_KEYS]; + Oid conffeqop[INDEX_MAX_KEYS]; + int numfkdelsetcols; + AttrNumber confdelsetcols[INDEX_MAX_KEYS]; + Relation refdRel; - createForeignKeyActionTriggers(partRel, conform->confrelid, - fkconstraint, fk->conoid, - conform->conindid, - InvalidOid, InvalidOid, - NULL, NULL); + DeconstructFkConstraintRow(contup, + &numfks, + conkey, + confkey, + conpfeqop, + conppeqop, + conffeqop, + &numfkdelsetcols, + confdelsetcols); + + /* Create a synthetic node we'll use throughout */ + fkconstraint = makeNode(Constraint); + fkconstraint->contype = CONSTRAINT_FOREIGN; + fkconstraint->conname = pstrdup(NameStr(conform->conname)); + fkconstraint->deferrable = conform->condeferrable; + fkconstraint->initdeferred = conform->condeferred; + fkconstraint->skip_validation = true; + fkconstraint->initially_valid = true; + /* a few irrelevant fields omitted here */ + fkconstraint->pktable = NULL; + fkconstraint->fk_attrs = NIL; + fkconstraint->pk_attrs = NIL; + fkconstraint->fk_matchtype = conform->confmatchtype; + fkconstraint->fk_upd_action = conform->confupdtype; + fkconstraint->fk_del_action = conform->confdeltype; + fkconstraint->fk_del_set_cols = NIL; + fkconstraint->old_conpfeqop = NIL; + fkconstraint->old_pktable_oid = InvalidOid; + fkconstraint->location = -1; + + attmap = build_attrmap_by_name(RelationGetDescr(partRel), + RelationGetDescr(rel), + false); + for (int i = 0; i < numfks; i++) + { + Form_pg_attribute att; + + att = TupleDescAttr(RelationGetDescr(partRel), + attmap->attnums[conkey[i] - 1] - 1); + fkconstraint->fk_attrs = lappend(fkconstraint->fk_attrs, + makeString(NameStr(att->attname))); + } + + refdRel = table_open(fk->confrelid, AccessShareLock); + + addFkRecurseReferenced(wqueue, fkconstraint, partRel, + refdRel, + conform->conindid, + fk->conoid, + numfks, + confkey, + conkey, + conpfeqop, + conppeqop, + conffeqop, + numfkdelsetcols, + confdelsetcols, + true, + InvalidOid, InvalidOid, + conform->conperiod); + table_close(refdRel, AccessShareLock); + } ReleaseSysCache(contup); + ReleaseSysCache(parentConTup); } list_free_deep(fks); if (trigrel) @@ -19549,7 +19711,7 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, * completion; this completes the detaching process. */ static ObjectAddress -ATExecDetachPartitionFinalize(Relation rel, RangeVar *name) +ATExecDetachPartitionFinalize(List **wqueue, Relation rel, RangeVar *name) { Relation partRel; ObjectAddress address; @@ -19567,7 +19729,7 @@ ATExecDetachPartitionFinalize(Relation rel, RangeVar *name) */ WaitForOlderSnapshots(snap->xmin, false); - DetachPartitionFinalize(rel, partRel, true, InvalidOid); + DetachPartitionFinalize(wqueue, rel, partRel, true, InvalidOid); ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel)); diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 8c04a24b37d..b73e7dced8f 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -2934,3 +2934,99 @@ DETAIL: drop cascades to table fkpart11.pk drop cascades to table fkpart11.fk_parted drop cascades to table fkpart11.fk_another drop cascades to function fkpart11.print_row() +-- When a table is attached as partition to a partitioned table that has +-- a foreign key to another partitioned table, it acquires a clone of the +-- FK. Upon detach, this clone is not removed, but instead becomes an +-- independent FK. If it then attaches to the partitioned table again, +-- the FK from the parent "takes over" ownership of the independent FK rather +-- than creating a separate one. +CREATE SCHEMA fkpart12 + CREATE TABLE fk_p ( id int, jd int, PRIMARY KEY(id, jd)) PARTITION BY list (id) + CREATE TABLE fk_p_1 PARTITION OF fk_p FOR VALUES IN (1) PARTITION BY list (jd) + CREATE TABLE fk_p_1_1 PARTITION OF fk_p_1 FOR VALUES IN (1) + CREATE TABLE fk_p_1_2 PARTITION OF fk_p_1 FOR VALUES IN (2) + CREATE TABLE fk_p_2 PARTITION OF fk_p FOR VALUES IN (2) PARTITION BY list (jd) + CREATE TABLE fk_p_2_1 PARTITION OF fk_p_2 FOR VALUES IN (1) + CREATE TABLE fk_p_2_2 PARTITION OF fk_p_2 FOR VALUES IN (2) + CREATE TABLE fk_r_1 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) + CREATE TABLE fk_r_2 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) PARTITION BY list (id) + CREATE TABLE fk_r_2_1 PARTITION OF fk_r_2 FOR VALUES IN (2, 1) + CREATE TABLE fk_r ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL, + FOREIGN KEY (p_id, p_jd) REFERENCES fk_p (id, jd) + ) PARTITION BY list (id); +SET search_path TO fkpart12; +INSERT INTO fk_p VALUES (1, 1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2); +\d fk_r_2 + Partitioned table "fkpart12.fk_r_2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + id | integer | | not null | + p_id | integer | | not null | + p_jd | integer | | not null | +Partition of: fk_r FOR VALUES IN (2) +Partition key: LIST (id) +Indexes: + "fk_r_2_pkey" PRIMARY KEY, btree (id) +Foreign-key constraints: + TABLE "fk_r" CONSTRAINT "fk_r_p_id_p_jd_fkey" FOREIGN KEY (p_id, p_jd) REFERENCES fk_p(id, jd) +Number of partitions: 1 (Use \d+ to list them.) + +INSERT INTO fk_r VALUES (1, 1, 1); +INSERT INTO fk_r VALUES (2, 2, 1); +ERROR: insert or update on table "fk_r_2_1" violates foreign key constraint "fk_r_p_id_p_jd_fkey" +DETAIL: Key (p_id, p_jd)=(2, 1) is not present in table "fk_p". +ALTER TABLE fk_r DETACH PARTITION fk_r_1; +ALTER TABLE fk_r DETACH PARTITION fk_r_2; +\d fk_r_2 + Partitioned table "fkpart12.fk_r_2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + id | integer | | not null | + p_id | integer | | not null | + p_jd | integer | | not null | +Partition key: LIST (id) +Indexes: + "fk_r_2_pkey" PRIMARY KEY, btree (id) +Foreign-key constraints: + "fk_r_p_id_p_jd_fkey" FOREIGN KEY (p_id, p_jd) REFERENCES fk_p(id, jd) +Number of partitions: 1 (Use \d+ to list them.) + +INSERT INTO fk_r_1 VALUES (2, 1, 2); -- should fail +ERROR: insert or update on table "fk_r_1" violates foreign key constraint "fk_r_p_id_p_jd_fkey" +DETAIL: Key (p_id, p_jd)=(1, 2) is not present in table "fk_p". +DELETE FROM fk_p; -- should fail +ERROR: update or delete on table "fk_p_1_1" violates foreign key constraint "fk_r_1_p_id_p_jd_fkey1" on table "fk_r_1" +DETAIL: Key (id, jd)=(1, 1) is still referenced from table "fk_r_1". +ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2); +\d fk_r_2 + Partitioned table "fkpart12.fk_r_2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + id | integer | | not null | + p_id | integer | | not null | + p_jd | integer | | not null | +Partition of: fk_r FOR VALUES IN (2) +Partition key: LIST (id) +Indexes: + "fk_r_2_pkey" PRIMARY KEY, btree (id) +Foreign-key constraints: + TABLE "fk_r" CONSTRAINT "fk_r_p_id_p_jd_fkey" FOREIGN KEY (p_id, p_jd) REFERENCES fk_p(id, jd) +Number of partitions: 1 (Use \d+ to list them.) + +DELETE FROM fk_p; -- should fail +ERROR: update or delete on table "fk_p_1_1" violates foreign key constraint "fk_r_p_id_p_jd_fkey2" on table "fk_r" +DETAIL: Key (id, jd)=(1, 1) is still referenced from table "fk_r". +-- these should all fail +ALTER TABLE fk_r_1 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; +ERROR: cannot drop inherited constraint "fk_r_p_id_p_jd_fkey" of relation "fk_r_1" +ALTER TABLE fk_r DROP CONSTRAINT fk_r_p_id_p_jd_fkey1; +ERROR: cannot drop inherited constraint "fk_r_p_id_p_jd_fkey1" of relation "fk_r" +ALTER TABLE fk_r_2 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; +ERROR: cannot drop inherited constraint "fk_r_p_id_p_jd_fkey" of relation "fk_r_2" +SET client_min_messages TO warning; +DROP SCHEMA fkpart12 CASCADE; +RESET client_min_messages; +RESET search_path; diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index d1aac5357f0..9b2a6b6bff7 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -2086,3 +2086,60 @@ UPDATE fkpart11.pk SET a = 3 WHERE a = 4; UPDATE fkpart11.pk SET a = 1 WHERE a = 2; DROP SCHEMA fkpart11 CASCADE; + +-- When a table is attached as partition to a partitioned table that has +-- a foreign key to another partitioned table, it acquires a clone of the +-- FK. Upon detach, this clone is not removed, but instead becomes an +-- independent FK. If it then attaches to the partitioned table again, +-- the FK from the parent "takes over" ownership of the independent FK rather +-- than creating a separate one. +CREATE SCHEMA fkpart12 + CREATE TABLE fk_p ( id int, jd int, PRIMARY KEY(id, jd)) PARTITION BY list (id) + CREATE TABLE fk_p_1 PARTITION OF fk_p FOR VALUES IN (1) PARTITION BY list (jd) + CREATE TABLE fk_p_1_1 PARTITION OF fk_p_1 FOR VALUES IN (1) + CREATE TABLE fk_p_1_2 PARTITION OF fk_p_1 FOR VALUES IN (2) + CREATE TABLE fk_p_2 PARTITION OF fk_p FOR VALUES IN (2) PARTITION BY list (jd) + CREATE TABLE fk_p_2_1 PARTITION OF fk_p_2 FOR VALUES IN (1) + CREATE TABLE fk_p_2_2 PARTITION OF fk_p_2 FOR VALUES IN (2) + CREATE TABLE fk_r_1 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) + CREATE TABLE fk_r_2 ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL) PARTITION BY list (id) + CREATE TABLE fk_r_2_1 PARTITION OF fk_r_2 FOR VALUES IN (2, 1) + CREATE TABLE fk_r ( id int PRIMARY KEY, p_id int NOT NULL, p_jd int NOT NULL, + FOREIGN KEY (p_id, p_jd) REFERENCES fk_p (id, jd) + ) PARTITION BY list (id); +SET search_path TO fkpart12; + +INSERT INTO fk_p VALUES (1, 1); + +ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2); + +\d fk_r_2 + +INSERT INTO fk_r VALUES (1, 1, 1); +INSERT INTO fk_r VALUES (2, 2, 1); + +ALTER TABLE fk_r DETACH PARTITION fk_r_1; +ALTER TABLE fk_r DETACH PARTITION fk_r_2; + +\d fk_r_2 + +INSERT INTO fk_r_1 VALUES (2, 1, 2); -- should fail +DELETE FROM fk_p; -- should fail + +ALTER TABLE fk_r ATTACH PARTITION fk_r_1 FOR VALUES IN (1); +ALTER TABLE fk_r ATTACH PARTITION fk_r_2 FOR VALUES IN (2); + +\d fk_r_2 + +DELETE FROM fk_p; -- should fail + +-- these should all fail +ALTER TABLE fk_r_1 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; +ALTER TABLE fk_r DROP CONSTRAINT fk_r_p_id_p_jd_fkey1; +ALTER TABLE fk_r_2 DROP CONSTRAINT fk_r_p_id_p_jd_fkey; + +SET client_min_messages TO warning; +DROP SCHEMA fkpart12 CASCADE; +RESET client_min_messages; +RESET search_path; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 57de1acff3a..49a0d58bd98 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3271,6 +3271,7 @@ _stringlist access_vector_t acquireLocksOnSubLinks_context add_nulling_relids_context +addFkConstraintSides adjust_appendrel_attrs_context allocfunc amadjustmembers_function -- 2.39.5 --mr6adwpq5l3ngngx--