Skip to content

Commit 6f70d7c

Browse files
committed
Have ALTER CONSTRAINT recurse on partitioned tables
When ALTER TABLE .. ALTER CONSTRAINT changes deferrability properties changed in a partitioned table, we failed to propagate those changes correctly to partitions and to triggers. Repair by adding a recursion mechanism to affect all derived constraints and all derived triggers. (In particular, recurse to partitions even if their respective parents are already in the desired state: it is possible for the partitions to have been altered individually.) Because foreign keys involve tables in two sides, we cannot use the standard ALTER TABLE recursion mechanism, so we invent our own by following pg_constraint.conparentid down. When ALTER TABLE .. ALTER CONSTRAINT is invoked on the derived pg_constraint object that's automaticaly created in a partition as a result of a constraint added to its parent, raise an error instead of pretending to work and then failing to modify all the affected triggers. Before this commit such a command would be allowed but failed to affect all triggers, so it would silently misbehave. (Restoring dumps of existing databases is not affected, because pg_dump does not produce anything for such a derived constraint anyway.) Add some tests for the case. Backpatch to 11, where foreign key support was added to partitioned tables by commit 3de241d. (A related change is commit f56f8f8 in pg12 which added support for FKs *referencing* partitioned tables; this is what forces us to use an ad-hoc recursion mechanism for this.) Diagnosed by Tom Lane from bug report from Ron L Johnson. As of this writing, no reviews were offered. Discussion: https://postgr.es/m/75fe0761-a291-86a9-c8d8-4906da077469@gmail.com Discussion: https://postgr.es/m/3144850.1607369633@sss.pgh.pa.us
1 parent f33a178 commit 6f70d7c

File tree

3 files changed

+311
-38
lines changed

3 files changed

+311
-38
lines changed

src/backend/commands/tablecmds.c

Lines changed: 158 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,9 @@ static void AlterSeqNamespaces(Relation classRel, Relation rel,
357357
LOCKMODE lockmode);
358358
static ObjectAddress ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd,
359359
bool recurse, bool recursing, LOCKMODE lockmode);
360+
static bool ATExecAlterConstrRecurse(Constraint *cmdcon, Relation conrel, Relation tgrel,
361+
Relation rel, HeapTuple contuple, List **otherrelids,
362+
LOCKMODE lockmode);
360363
static ObjectAddress ATExecValidateConstraint(List **wqueue,
361364
Relation rel, char *constrName,
362365
bool recurse, bool recursing, LOCKMODE lockmode);
@@ -4669,6 +4672,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
46694672
break;
46704673
case AT_AlterConstraint: /* ALTER CONSTRAINT */
46714674
ATSimplePermissions(rel, ATT_TABLE);
4675+
/* Recursion occurs during execution phase */
46724676
pass = AT_PASS_MISC;
46734677
break;
46744678
case AT_ValidateConstraint: /* VALIDATE CONSTRAINT */
@@ -10190,28 +10194,29 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk,
1019010194
* Update the attributes of a constraint.
1019110195
*
1019210196
* Currently only works for Foreign Key constraints.
10193-
* Foreign keys do not inherit, so we purposely ignore the
10194-
* recursion bit here, but we keep the API the same for when
10195-
* other constraint types are supported.
1019610197
*
1019710198
* If the constraint is modified, returns its address; otherwise, return
1019810199
* InvalidObjectAddress.
1019910200
*/
1020010201
static ObjectAddress
10201-
ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd,
10202-
bool recurse, bool recursing, LOCKMODE lockmode)
10202+
ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, bool recurse,
10203+
bool recursing, LOCKMODE lockmode)
1020310204
{
1020410205
Constraint *cmdcon;
1020510206
Relation conrel;
10207+
Relation tgrel;
1020610208
SysScanDesc scan;
1020710209
ScanKeyData skey[3];
1020810210
HeapTuple contuple;
1020910211
Form_pg_constraint currcon;
1021010212
ObjectAddress address;
10213+
List *otherrelids = NIL;
10214+
ListCell *lc;
1021110215

1021210216
cmdcon = castNode(Constraint, cmd->def);
1021310217

1021410218
conrel = table_open(ConstraintRelationId, RowExclusiveLock);
10219+
tgrel = table_open(TriggerRelationId, RowExclusiveLock);
1021510220

1021610221
/*
1021710222
* Find and check the target constraint
@@ -10245,50 +10250,150 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd,
1024510250
errmsg("constraint \"%s\" of relation \"%s\" is not a foreign key constraint",
1024610251
cmdcon->conname, RelationGetRelationName(rel))));
1024710252

10253+
/*
10254+
* If it's not the topmost constraint, raise an error.
10255+
*
10256+
* Altering a non-topmost constraint leaves some triggers untouched, since
10257+
* they are not directly connected to this constraint; also, pg_dump would
10258+
* ignore the deferrability status of the individual constraint, since it
10259+
* only dumps topmost constraints. Avoid these problems by refusing this
10260+
* operation and telling the user to alter the parent constraint instead.
10261+
*/
10262+
if (OidIsValid(currcon->conparentid))
10263+
{
10264+
HeapTuple tp;
10265+
Oid parent = currcon->conparentid;
10266+
char *ancestorname = NULL;
10267+
char *ancestortable = NULL;
10268+
10269+
/* Loop to find the topmost constraint */
10270+
while (HeapTupleIsValid(tp = SearchSysCache1(CONSTROID, ObjectIdGetDatum(parent))))
10271+
{
10272+
Form_pg_constraint contup = (Form_pg_constraint) GETSTRUCT(tp);
10273+
10274+
/* If no parent, this is the constraint we want */
10275+
if (!OidIsValid(contup->conparentid))
10276+
{
10277+
ancestorname = pstrdup(NameStr(contup->conname));
10278+
ancestortable = get_rel_name(contup->conrelid);
10279+
ReleaseSysCache(tp);
10280+
break;
10281+
}
10282+
10283+
parent = contup->conparentid;
10284+
ReleaseSysCache(tp);
10285+
}
10286+
10287+
ereport(ERROR,
10288+
(errmsg("cannot alter constraint \"%s\" on relation \"%s\"",
10289+
cmdcon->conname, RelationGetRelationName(rel)),
10290+
ancestorname && ancestortable ?
10291+
errdetail("Constraint \"%s\" is derived from constraint \"%s\" of relation \"%s\".",
10292+
cmdcon->conname, ancestorname, ancestortable) : 0,
10293+
errhint("You may alter the constraint it derives from, instead.")));
10294+
}
10295+
10296+
/*
10297+
* Do the actual catalog work. We can skip changing if already in the
10298+
* desired state, but not if a partitioned table: partitions need to be
10299+
* processed regardless, in case they had the constraint locally changed.
10300+
*/
10301+
address = InvalidObjectAddress;
10302+
if (currcon->condeferrable != cmdcon->deferrable ||
10303+
currcon->condeferred != cmdcon->initdeferred ||
10304+
rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
10305+
{
10306+
if (ATExecAlterConstrRecurse(cmdcon, conrel, tgrel, rel, contuple,
10307+
&otherrelids, lockmode))
10308+
ObjectAddressSet(address, ConstraintRelationId, currcon->oid);
10309+
}
10310+
10311+
/*
10312+
* ATExecConstrRecurse already invalidated relcache for the relations
10313+
* having the constraint itself; here we also invalidate for relations
10314+
* that have any triggers that are part of the constraint.
10315+
*/
10316+
foreach(lc, otherrelids)
10317+
CacheInvalidateRelcacheByRelid(lfirst_oid(lc));
10318+
10319+
systable_endscan(scan);
10320+
10321+
table_close(tgrel, RowExclusiveLock);
10322+
table_close(conrel, RowExclusiveLock);
10323+
10324+
return address;
10325+
}
10326+
10327+
/*
10328+
* Recursive subroutine of ATExecAlterConstraint. Returns true if the
10329+
* constraint is altered.
10330+
*
10331+
* *otherrelids is appended OIDs of relations containing affected triggers.
10332+
*
10333+
* Note that we must recurse even when the values are correct, in case
10334+
* indirect descendants have had their constraints altered locally.
10335+
* (This could be avoided if we forbade altering constraints in partitions
10336+
* but existing releases don't do that.)
10337+
*/
10338+
static bool
10339+
ATExecAlterConstrRecurse(Constraint *cmdcon, Relation conrel, Relation tgrel,
10340+
Relation rel, HeapTuple contuple, List **otherrelids,
10341+
LOCKMODE lockmode)
10342+
{
10343+
Form_pg_constraint currcon;
10344+
Oid conoid;
10345+
Oid refrelid;
10346+
bool changed = false;
10347+
10348+
currcon = (Form_pg_constraint) GETSTRUCT(contuple);
10349+
conoid = currcon->oid;
10350+
refrelid = currcon->confrelid;
10351+
10352+
/*
10353+
* Update pg_constraint with the flags from cmdcon.
10354+
*
10355+
* If called to modify a constraint that's already in the desired state,
10356+
* silently do nothing.
10357+
*/
1024810358
if (currcon->condeferrable != cmdcon->deferrable ||
1024910359
currcon->condeferred != cmdcon->initdeferred)
1025010360
{
1025110361
HeapTuple copyTuple;
10252-
HeapTuple tgtuple;
1025310362
Form_pg_constraint copy_con;
10254-
List *otherrelids = NIL;
10363+
HeapTuple tgtuple;
1025510364
ScanKeyData tgkey;
1025610365
SysScanDesc tgscan;
10257-
Relation tgrel;
10258-
ListCell *lc;
1025910366

10260-
/*
10261-
* Now update the catalog, while we have the door open.
10262-
*/
1026310367
copyTuple = heap_copytuple(contuple);
1026410368
copy_con = (Form_pg_constraint) GETSTRUCT(copyTuple);
1026510369
copy_con->condeferrable = cmdcon->deferrable;
1026610370
copy_con->condeferred = cmdcon->initdeferred;
1026710371
CatalogTupleUpdate(conrel, &copyTuple->t_self, copyTuple);
1026810372

1026910373
InvokeObjectPostAlterHook(ConstraintRelationId,
10270-
currcon->oid, 0);
10374+
conoid, 0);
1027110375

1027210376
heap_freetuple(copyTuple);
10377+
changed = true;
10378+
10379+
/* Make new constraint flags visible to others */
10380+
CacheInvalidateRelcache(rel);
1027310381

1027410382
/*
1027510383
* Now we need to update the multiple entries in pg_trigger that
1027610384
* implement the constraint.
1027710385
*/
10278-
tgrel = table_open(TriggerRelationId, RowExclusiveLock);
10279-
1028010386
ScanKeyInit(&tgkey,
1028110387
Anum_pg_trigger_tgconstraint,
1028210388
BTEqualStrategyNumber, F_OIDEQ,
10283-
ObjectIdGetDatum(currcon->oid));
10284-
10389+
ObjectIdGetDatum(conoid));
1028510390
tgscan = systable_beginscan(tgrel, TriggerConstraintIndexId, true,
1028610391
NULL, 1, &tgkey);
10287-
1028810392
while (HeapTupleIsValid(tgtuple = systable_getnext(tgscan)))
1028910393
{
1029010394
Form_pg_trigger tgform = (Form_pg_trigger) GETSTRUCT(tgtuple);
1029110395
Form_pg_trigger copy_tg;
10396+
HeapTuple copyTuple;
1029210397

1029310398
/*
1029410399
* Remember OIDs of other relation(s) involved in FK constraint.
@@ -10297,8 +10402,8 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd,
1029710402
* change, but let's be conservative.)
1029810403
*/
1029910404
if (tgform->tgrelid != RelationGetRelid(rel))
10300-
otherrelids = list_append_unique_oid(otherrelids,
10301-
tgform->tgrelid);
10405+
*otherrelids = list_append_unique_oid(*otherrelids,
10406+
tgform->tgrelid);
1030210407

1030310408
/*
1030410409
* Update deferrability of RI_FKey_noaction_del,
@@ -10325,31 +10430,46 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd,
1032510430
}
1032610431

1032710432
systable_endscan(tgscan);
10433+
}
1032810434

10329-
table_close(tgrel, RowExclusiveLock);
10435+
/*
10436+
* If the table at either end of the constraint is partitioned, we need to
10437+
* recurse and handle every constraint that is a child of this one.
10438+
*
10439+
* (This assumes that the recurse flag is forcibly set for partitioned
10440+
* tables, and not set for legacy inheritance, though we don't check for
10441+
* that here.)
10442+
*/
10443+
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ||
10444+
get_rel_relkind(refrelid) == RELKIND_PARTITIONED_TABLE)
10445+
{
10446+
ScanKeyData pkey;
10447+
SysScanDesc pscan;
10448+
HeapTuple childtup;
1033010449

10331-
/*
10332-
* Invalidate relcache so that others see the new attributes. We must
10333-
* inval both the named rel and any others having relevant triggers.
10334-
* (At present there should always be exactly one other rel, but
10335-
* there's no need to hard-wire such an assumption here.)
10336-
*/
10337-
CacheInvalidateRelcache(rel);
10338-
foreach(lc, otherrelids)
10450+
ScanKeyInit(&pkey,
10451+
Anum_pg_constraint_conparentid,
10452+
BTEqualStrategyNumber, F_OIDEQ,
10453+
ObjectIdGetDatum(conoid));
10454+
10455+
pscan = systable_beginscan(conrel, ConstraintParentIndexId,
10456+
true, NULL, 1, &pkey);
10457+
10458+
while (HeapTupleIsValid(childtup = systable_getnext(pscan)))
1033910459
{
10340-
CacheInvalidateRelcacheByRelid(lfirst_oid(lc));
10460+
Form_pg_constraint childcon = (Form_pg_constraint) GETSTRUCT(childtup);
10461+
Relation childrel;
10462+
10463+
childrel = table_open(childcon->conrelid, lockmode);
10464+
ATExecAlterConstrRecurse(cmdcon, conrel, tgrel, childrel, childtup,
10465+
otherrelids, lockmode);
10466+
table_close(childrel, NoLock);
1034110467
}
1034210468

10343-
ObjectAddressSet(address, ConstraintRelationId, currcon->oid);
10469+
systable_endscan(pscan);
1034410470
}
10345-
else
10346-
address = InvalidObjectAddress;
1034710471

10348-
systable_endscan(scan);
10349-
10350-
table_close(conrel, RowExclusiveLock);
10351-
10352-
return address;
10472+
return changed;
1035310473
}
1035410474

1035510475
/*

src/test/regress/expected/foreign_key.out

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2313,6 +2313,84 @@ SET CONSTRAINTS fk_a_fkey DEFERRED;
23132313
DELETE FROM pk WHERE a = 1;
23142314
DELETE FROM fk WHERE a = 1;
23152315
COMMIT; -- OK
2316+
-- Verify constraint deferrability when changed by ALTER
2317+
-- Partitioned table at referencing end
2318+
CREATE TABLE pt(f1 int, f2 int, f3 int, PRIMARY KEY(f1,f2));
2319+
CREATE TABLE ref(f1 int, f2 int, f3 int)
2320+
PARTITION BY list(f1);
2321+
CREATE TABLE ref1 PARTITION OF ref FOR VALUES IN (1);
2322+
CREATE TABLE ref2 PARTITION OF ref FOR VALUES in (2);
2323+
ALTER TABLE ref ADD FOREIGN KEY(f1,f2) REFERENCES pt;
2324+
ALTER TABLE ref ALTER CONSTRAINT ref_f1_f2_fkey
2325+
DEFERRABLE INITIALLY DEFERRED;
2326+
INSERT INTO pt VALUES(1,2,3);
2327+
INSERT INTO ref VALUES(1,2,3);
2328+
BEGIN;
2329+
DELETE FROM pt;
2330+
DELETE FROM ref;
2331+
ABORT;
2332+
DROP TABLE pt, ref;
2333+
-- Multi-level partitioning at referencing end
2334+
CREATE TABLE pt(f1 int, f2 int, f3 int, PRIMARY KEY(f1,f2));
2335+
CREATE TABLE ref(f1 int, f2 int, f3 int)
2336+
PARTITION BY list(f1);
2337+
CREATE TABLE ref1_2 PARTITION OF ref FOR VALUES IN (1, 2) PARTITION BY list (f2);
2338+
CREATE TABLE ref1 PARTITION OF ref1_2 FOR VALUES IN (1);
2339+
CREATE TABLE ref2 PARTITION OF ref1_2 FOR VALUES IN (2) PARTITION BY list (f2);
2340+
CREATE TABLE ref22 PARTITION OF ref2 FOR VALUES IN (2);
2341+
ALTER TABLE ref ADD FOREIGN KEY(f1,f2) REFERENCES pt;
2342+
INSERT INTO pt VALUES(1,2,3);
2343+
INSERT INTO ref VALUES(1,2,3);
2344+
ALTER TABLE ref22 ALTER CONSTRAINT ref_f1_f2_fkey
2345+
DEFERRABLE INITIALLY IMMEDIATE; -- fails
2346+
ERROR: cannot alter constraint "ref_f1_f2_fkey" on relation "ref22"
2347+
DETAIL: Constraint "ref_f1_f2_fkey" is derived from constraint "ref_f1_f2_fkey" of relation "ref".
2348+
HINT: You may alter the constraint it derives from, instead.
2349+
ALTER TABLE ref ALTER CONSTRAINT ref_f1_f2_fkey
2350+
DEFERRABLE INITIALLY DEFERRED;
2351+
BEGIN;
2352+
DELETE FROM pt;
2353+
DELETE FROM ref;
2354+
ABORT;
2355+
DROP TABLE pt, ref;
2356+
-- Partitioned table at referenced end
2357+
CREATE TABLE pt(f1 int, f2 int, f3 int, PRIMARY KEY(f1,f2))
2358+
PARTITION BY LIST(f1);
2359+
CREATE TABLE pt1 PARTITION OF pt FOR VALUES IN (1);
2360+
CREATE TABLE pt2 PARTITION OF pt FOR VALUES IN (2);
2361+
CREATE TABLE ref(f1 int, f2 int, f3 int);
2362+
ALTER TABLE ref ADD FOREIGN KEY(f1,f2) REFERENCES pt;
2363+
ALTER TABLE ref ALTER CONSTRAINT ref_f1_f2_fkey
2364+
DEFERRABLE INITIALLY DEFERRED;
2365+
INSERT INTO pt VALUES(1,2,3);
2366+
INSERT INTO ref VALUES(1,2,3);
2367+
BEGIN;
2368+
DELETE FROM pt;
2369+
DELETE FROM ref;
2370+
ABORT;
2371+
DROP TABLE pt, ref;
2372+
-- Multi-level partitioning at at referenced end
2373+
CREATE TABLE pt(f1 int, f2 int, f3 int, PRIMARY KEY(f1,f2))
2374+
PARTITION BY LIST(f1);
2375+
CREATE TABLE pt1_2 PARTITION OF pt FOR VALUES IN (1, 2) PARTITION BY LIST (f1);
2376+
CREATE TABLE pt1 PARTITION OF pt1_2 FOR VALUES IN (1);
2377+
CREATE TABLE pt2 PARTITION OF pt1_2 FOR VALUES IN (2);
2378+
CREATE TABLE ref(f1 int, f2 int, f3 int);
2379+
ALTER TABLE ref ADD FOREIGN KEY(f1,f2) REFERENCES pt;
2380+
ALTER TABLE ref ALTER CONSTRAINT ref_f1_f2_fkey1
2381+
DEFERRABLE INITIALLY DEFERRED; -- fails
2382+
ERROR: cannot alter constraint "ref_f1_f2_fkey1" on relation "ref"
2383+
DETAIL: Constraint "ref_f1_f2_fkey1" is derived from constraint "ref_f1_f2_fkey" of relation "ref".
2384+
HINT: You may alter the constraint it derives from, instead.
2385+
ALTER TABLE ref ALTER CONSTRAINT ref_f1_f2_fkey
2386+
DEFERRABLE INITIALLY DEFERRED;
2387+
INSERT INTO pt VALUES(1,2,3);
2388+
INSERT INTO ref VALUES(1,2,3);
2389+
BEGIN;
2390+
DELETE FROM pt;
2391+
DELETE FROM ref;
2392+
ABORT;
2393+
DROP TABLE pt, ref;
23162394
DROP SCHEMA fkpart9 CASCADE;
23172395
NOTICE: drop cascades to 2 other objects
23182396
DETAIL: drop cascades to table pk

0 commit comments

Comments
 (0)