diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index a149ca044c..95308db67e 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -322,6 +322,9 @@ static void AlterSeqNamespaces(Relation classRel, Relation rel, LOCKMODE lockmode); static ObjectAddress ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, bool recurse, bool recursing, LOCKMODE lockmode); +static bool ATExecAlterConstrRecurse(Constraint *cmdcon, Relation conrel, + Relation tgrel, Relation rel, HeapTuple contuple, + List **otherrelids, LOCKMODE lockmode); static ObjectAddress ATExecValidateConstraint(List **wqueue, Relation rel, char *constrName, bool recurse, bool recursing, LOCKMODE lockmode); @@ -3953,6 +3956,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, break; case AT_AlterConstraint: /* ALTER CONSTRAINT */ ATSimplePermissions(rel, ATT_TABLE); + /* Recursion occurs during execution phase */ pass = AT_PASS_MISC; break; case AT_ValidateConstraint: /* VALIDATE CONSTRAINT */ @@ -8311,28 +8315,29 @@ CloneFkReferencing(Relation pg_constraint, Relation parentRel, * Update the attributes of a constraint. * * Currently only works for Foreign Key constraints. - * Foreign keys do not inherit, so we purposely ignore the - * recursion bit here, but we keep the API the same for when - * other constraint types are supported. * * If the constraint is modified, returns its address; otherwise, return * InvalidObjectAddress. */ static ObjectAddress -ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, - bool recurse, bool recursing, LOCKMODE lockmode) +ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, bool recurse, + bool recursing, LOCKMODE lockmode) { Constraint *cmdcon; Relation conrel; + Relation tgrel; SysScanDesc scan; ScanKeyData skey[3]; HeapTuple contuple; Form_pg_constraint currcon; ObjectAddress address; + List *otherrelids = NIL; + ListCell *lc; cmdcon = castNode(Constraint, cmd->def); conrel = heap_open(ConstraintRelationId, RowExclusiveLock); + tgrel = heap_open(TriggerRelationId, RowExclusiveLock); /* * Find and check the target constraint @@ -8366,17 +8371,118 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, errmsg("constraint \"%s\" of relation \"%s\" is not a foreign key constraint", cmdcon->conname, RelationGetRelationName(rel)))); + /* + * If it's not the topmost constraint, raise an error. + * + * Altering a non-topmost constraint leaves some triggers untouched, since + * they are not directly connected to this constraint; also, pg_dump would + * ignore the deferrability status of the individual constraint, since it + * only dumps topmost constraints. Avoid these problems by refusing this + * operation and telling the user to alter the parent constraint instead. + */ + if (OidIsValid(currcon->conparentid)) + { + HeapTuple tp; + Oid parent = currcon->conparentid; + char *ancestorname = NULL; + char *ancestortable = NULL; + + /* Loop to find the topmost constraint */ + while (HeapTupleIsValid(tp = SearchSysCache1(CONSTROID, ObjectIdGetDatum(parent)))) + { + Form_pg_constraint contup = (Form_pg_constraint) GETSTRUCT(tp); + + /* If no parent, this is the constraint we want */ + if (!OidIsValid(contup->conparentid)) + { + ancestorname = pstrdup(NameStr(contup->conname)); + ancestortable = get_rel_name(contup->conrelid); + ReleaseSysCache(tp); + break; + } + + parent = contup->conparentid; + ReleaseSysCache(tp); + } + + ereport(ERROR, + (errmsg("cannot alter constraint \"%s\" on relation \"%s\"", + cmdcon->conname, RelationGetRelationName(rel)), + ancestorname && ancestortable ? + errdetail("Constraint \"%s\" is derived from constraint \"%s\" of relation \"%s\".", + cmdcon->conname, ancestorname, ancestortable) : 0, + errhint("You may alter the constraint it derives from, instead."))); + } + + /* + * Do the actual catalog work. We can skip changing if already in the + * desired state, but not if a partitioned table: partitions need to be + * processed regardless, in case they've had it locally changed. + */ + address = InvalidObjectAddress; + if (currcon->condeferrable != cmdcon->deferrable || + currcon->condeferred != cmdcon->initdeferred || + rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + { + if (ATExecAlterConstrRecurse(cmdcon, conrel, tgrel, rel, contuple, + &otherrelids, lockmode)) + ObjectAddressSet(address, ConstraintRelationId, + HeapTupleGetOid(contuple)); + } + + /* + * ATExecConstrRecurse already invalidated relcache for the relations + * having the constraint itself; here we also invalidate for relations + * that have any triggers that implement the constraint. + */ + foreach(lc, otherrelids) + CacheInvalidateRelcacheByRelid(lfirst_oid(lc)); + + systable_endscan(scan); + + heap_close(conrel, RowExclusiveLock); + heap_close(tgrel, RowExclusiveLock); + + return address; +} + +/* + * Recursive subroutine of ATExecAlterConstraint. Returns true if the + * constraint is altered. + * + * *otherrelids is appended OIDs of relations containing affected triggers. + * + * Note that we must recurse even when the values are correct, in case + * indirect descendants have had their constraints altered locally. + * (This could be avoided if we forbade altering constraints in partitions + * but existing releases don't do that.) + */ +static bool +ATExecAlterConstrRecurse(Constraint *cmdcon, Relation conrel, Relation tgrel, + Relation rel, HeapTuple contuple, List **otherrelids, + LOCKMODE lockmode) +{ + Form_pg_constraint currcon; + Oid conoid; + bool changed = false; + + currcon = (Form_pg_constraint) GETSTRUCT(contuple); + conoid = HeapTupleGetOid(contuple); + + /* + * Update pg_constraint with the flags from cmdcon. + * + * If called to modify a constraint that's already in the desired state, + * silently do nothing. + */ if (currcon->condeferrable != cmdcon->deferrable || currcon->condeferred != cmdcon->initdeferred) { HeapTuple copyTuple; - HeapTuple tgtuple; Form_pg_constraint copy_con; - List *otherrelids = NIL; + HeapTuple tgtuple; ScanKeyData tgkey; SysScanDesc tgscan; - Relation tgrel; - ListCell *lc; /* * Now update the catalog, while we have the door open. @@ -8391,25 +8497,26 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, HeapTupleGetOid(contuple), 0); heap_freetuple(copyTuple); + changed = true; + + /* Make new constraint flags visible to others */ + CacheInvalidateRelcache(rel); /* * Now we need to update the multiple entries in pg_trigger that * implement the constraint. */ - tgrel = heap_open(TriggerRelationId, RowExclusiveLock); - ScanKeyInit(&tgkey, Anum_pg_trigger_tgconstraint, BTEqualStrategyNumber, F_OIDEQ, ObjectIdGetDatum(HeapTupleGetOid(contuple))); - tgscan = systable_beginscan(tgrel, TriggerConstraintIndexId, true, NULL, 1, &tgkey); - while (HeapTupleIsValid(tgtuple = systable_getnext(tgscan))) { Form_pg_trigger tgform = (Form_pg_trigger) GETSTRUCT(tgtuple); Form_pg_trigger copy_tg; + HeapTuple copyTuple; /* * Remember OIDs of other relation(s) involved in FK constraint. @@ -8418,8 +8525,8 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, * change, but let's be conservative.) */ if (tgform->tgrelid != RelationGetRelid(rel)) - otherrelids = list_append_unique_oid(otherrelids, - tgform->tgrelid); + *otherrelids = list_append_unique_oid(*otherrelids, + tgform->tgrelid); /* * Update deferrability of RI_FKey_noaction_del, @@ -8447,32 +8554,45 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, } systable_endscan(tgscan); + } - heap_close(tgrel, RowExclusiveLock); + /* + * If the referencing table is partitioned, we need to recurse and handle + * every constraint that is a child of this one. + * + * (This assumes that the recurse flag is forcibly set for partitioned + * tables, and not set for legacy inheritance, though we don't check for + * that here.) + */ + if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + { + ScanKeyData pkey; + SysScanDesc pscan; + HeapTuple childtup; - /* - * Invalidate relcache so that others see the new attributes. We must - * inval both the named rel and any others having relevant triggers. - * (At present there should always be exactly one other rel, but - * there's no need to hard-wire such an assumption here.) - */ - CacheInvalidateRelcache(rel); - foreach(lc, otherrelids) + ScanKeyInit(&pkey, + Anum_pg_constraint_conparentid, + BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(conoid)); + + pscan = systable_beginscan(conrel, ConstraintParentIndexId, + true, NULL, 1, &pkey); + + while (HeapTupleIsValid(childtup = systable_getnext(pscan))) { - CacheInvalidateRelcacheByRelid(lfirst_oid(lc)); + Form_pg_constraint childcon = (Form_pg_constraint) GETSTRUCT(childtup); + Relation childrel; + + childrel = heap_open(childcon->conrelid, lockmode); + ATExecAlterConstrRecurse(cmdcon, conrel, tgrel, childrel, childtup, + otherrelids, lockmode); + heap_close(childrel, NoLock); } - ObjectAddressSet(address, ConstraintRelationId, - HeapTupleGetOid(contuple)); + systable_endscan(pscan); } - else - address = InvalidObjectAddress; - systable_endscan(scan); - - heap_close(conrel, RowExclusiveLock); - - return address; + return changed; } /* diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index ef1b2bc97d..34c0fd21a3 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -1954,7 +1954,50 @@ INSERT INTO fkpart8.tbl2 VALUES(1); ALTER TABLE fkpart8.tbl2 DROP CONSTRAINT tbl2_f1_fkey; ERROR: cannot ALTER TABLE "tbl2_p1" because it has pending trigger events COMMIT; +CREATE SCHEMA fkpart9; +SET search_path TO fkpart9; +-- Verify constraint deferrability when changed by ALTER +-- Partitioned table at referencing end +CREATE TABLE pt(f1 int, f2 int, f3 int, PRIMARY KEY(f1,f2)); +CREATE TABLE ref(f1 int, f2 int, f3 int) + PARTITION BY list(f1); +CREATE TABLE ref1 PARTITION OF ref FOR VALUES IN (1); +CREATE TABLE ref2 PARTITION OF ref FOR VALUES in (2); +ALTER TABLE ref ADD FOREIGN KEY(f1,f2) REFERENCES pt; +ALTER TABLE ref ALTER CONSTRAINT ref_f1_fkey + DEFERRABLE INITIALLY DEFERRED; +INSERT INTO pt VALUES(1,2,3); +INSERT INTO ref VALUES(1,2,3); +BEGIN; +DELETE FROM pt; +DELETE FROM ref; +ABORT; +DROP TABLE pt, ref; +-- Multi-level partitioning at referencing end +CREATE TABLE pt(f1 int, f2 int, f3 int, PRIMARY KEY(f1,f2)); +CREATE TABLE ref(f1 int, f2 int, f3 int) + PARTITION BY list(f1); +CREATE TABLE ref1_2 PARTITION OF ref FOR VALUES IN (1, 2) PARTITION BY list (f2); +CREATE TABLE ref1 PARTITION OF ref1_2 FOR VALUES IN (1); +CREATE TABLE ref2 PARTITION OF ref1_2 FOR VALUES IN (2) PARTITION BY list (f2); +CREATE TABLE ref22 PARTITION OF ref2 FOR VALUES IN (2); +ALTER TABLE ref ADD FOREIGN KEY(f1,f2) REFERENCES pt; +INSERT INTO pt VALUES(1,2,3); +INSERT INTO ref VALUES(1,2,3); +ALTER TABLE ref22 ALTER CONSTRAINT ref_f1_fkey + DEFERRABLE INITIALLY IMMEDIATE; -- fails +ERROR: cannot alter constraint "ref_f1_fkey" on relation "ref22" +DETAIL: Constraint "ref_f1_fkey" is derived from constraint "ref_f1_fkey" of relation "ref". +HINT: You may alter the constraint it derives from, instead. +ALTER TABLE ref ALTER CONSTRAINT ref_f1_fkey + DEFERRABLE INITIALLY DEFERRED; +BEGIN; +DELETE FROM pt; +DELETE FROM ref; +ABORT; +DROP TABLE pt, ref; +RESET search_path; \set VERBOSITY terse \\ -- suppress cascade details -drop schema fkpart0, fkpart1, fkpart2, fkpart3, fkpart8 cascade; +drop schema fkpart0, fkpart1, fkpart2, fkpart3, fkpart8, fkpart9 cascade; NOTICE: drop cascades to 12 other objects \set VERBOSITY default diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index 2ddda959ac..912f5ba686 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -1409,6 +1409,47 @@ INSERT INTO fkpart8.tbl2 VALUES(1); ALTER TABLE fkpart8.tbl2 DROP CONSTRAINT tbl2_f1_fkey; COMMIT; +CREATE SCHEMA fkpart9; +SET search_path TO fkpart9; +-- Verify constraint deferrability when changed by ALTER +-- Partitioned table at referencing end +CREATE TABLE pt(f1 int, f2 int, f3 int, PRIMARY KEY(f1,f2)); +CREATE TABLE ref(f1 int, f2 int, f3 int) + PARTITION BY list(f1); +CREATE TABLE ref1 PARTITION OF ref FOR VALUES IN (1); +CREATE TABLE ref2 PARTITION OF ref FOR VALUES in (2); +ALTER TABLE ref ADD FOREIGN KEY(f1,f2) REFERENCES pt; +ALTER TABLE ref ALTER CONSTRAINT ref_f1_fkey + DEFERRABLE INITIALLY DEFERRED; +INSERT INTO pt VALUES(1,2,3); +INSERT INTO ref VALUES(1,2,3); +BEGIN; +DELETE FROM pt; +DELETE FROM ref; +ABORT; +DROP TABLE pt, ref; +-- Multi-level partitioning at referencing end +CREATE TABLE pt(f1 int, f2 int, f3 int, PRIMARY KEY(f1,f2)); +CREATE TABLE ref(f1 int, f2 int, f3 int) + PARTITION BY list(f1); +CREATE TABLE ref1_2 PARTITION OF ref FOR VALUES IN (1, 2) PARTITION BY list (f2); +CREATE TABLE ref1 PARTITION OF ref1_2 FOR VALUES IN (1); +CREATE TABLE ref2 PARTITION OF ref1_2 FOR VALUES IN (2) PARTITION BY list (f2); +CREATE TABLE ref22 PARTITION OF ref2 FOR VALUES IN (2); +ALTER TABLE ref ADD FOREIGN KEY(f1,f2) REFERENCES pt; +INSERT INTO pt VALUES(1,2,3); +INSERT INTO ref VALUES(1,2,3); +ALTER TABLE ref22 ALTER CONSTRAINT ref_f1_fkey + DEFERRABLE INITIALLY IMMEDIATE; -- fails +ALTER TABLE ref ALTER CONSTRAINT ref_f1_fkey + DEFERRABLE INITIALLY DEFERRED; +BEGIN; +DELETE FROM pt; +DELETE FROM ref; +ABORT; +DROP TABLE pt, ref; +RESET search_path; + \set VERBOSITY terse \\ -- suppress cascade details -drop schema fkpart0, fkpart1, fkpart2, fkpart3, fkpart8 cascade; +drop schema fkpart0, fkpart1, fkpart2, fkpart3, fkpart8, fkpart9 cascade; \set VERBOSITY default