diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index a992f1bef8..72aba2ff6b 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -357,6 +357,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); @@ -4669,6 +4672,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 */ @@ -10190,28 +10194,29 @@ tryAttachPartitionForeignKey(ForeignKeyCacheInfo *fk, * 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 = table_open(ConstraintRelationId, RowExclusiveLock); + tgrel = table_open(TriggerRelationId, RowExclusiveLock); /* * Find and check the target constraint @@ -10245,21 +10250,120 @@ 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 had the constraint 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, currcon->oid); + } + + /* + * ATExecConstrRecurse already invalidated relcache for the relations + * having the constraint itself; here we also invalidate for relations + * that have any triggers that are part of the constraint. + */ + foreach(lc, otherrelids) + CacheInvalidateRelcacheByRelid(lfirst_oid(lc)); + + systable_endscan(scan); + + table_close(tgrel, RowExclusiveLock); + table_close(conrel, 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; + Oid refrelid; + bool changed = false; + + currcon = (Form_pg_constraint) GETSTRUCT(contuple); + conoid = currcon->oid; + refrelid = currcon->confrelid; + + /* + * 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. - */ copyTuple = heap_copytuple(contuple); copy_con = (Form_pg_constraint) GETSTRUCT(copyTuple); copy_con->condeferrable = cmdcon->deferrable; @@ -10267,28 +10371,29 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, CatalogTupleUpdate(conrel, ©Tuple->t_self, copyTuple); InvokeObjectPostAlterHook(ConstraintRelationId, - currcon->oid, 0); + conoid, 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 = table_open(TriggerRelationId, RowExclusiveLock); - ScanKeyInit(&tgkey, Anum_pg_trigger_tgconstraint, BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(currcon->oid)); - + ObjectIdGetDatum(conoid)); 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. @@ -10297,8 +10402,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, @@ -10325,31 +10430,46 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, } systable_endscan(tgscan); + } - table_close(tgrel, RowExclusiveLock); + /* + * If the table at either end of the constraint 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 || + get_rel_relkind(refrelid) == 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 = table_open(childcon->conrelid, lockmode); + ATExecAlterConstrRecurse(cmdcon, conrel, tgrel, childrel, childtup, + otherrelids, lockmode); + table_close(childrel, NoLock); } - ObjectAddressSet(address, ConstraintRelationId, currcon->oid); + systable_endscan(pscan); } - else - address = InvalidObjectAddress; - systable_endscan(scan); - - table_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 7386f4d635..bf794dce9d 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -2313,6 +2313,84 @@ SET CONSTRAINTS fk_a_fkey DEFERRED; DELETE FROM pk WHERE a = 1; DELETE FROM fk WHERE a = 1; COMMIT; -- OK +-- 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_f2_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_f2_fkey + DEFERRABLE INITIALLY IMMEDIATE; -- fails +ERROR: cannot alter constraint "ref_f1_f2_fkey" on relation "ref22" +DETAIL: Constraint "ref_f1_f2_fkey" is derived from constraint "ref_f1_f2_fkey" of relation "ref". +HINT: You may alter the constraint it derives from, instead. +ALTER TABLE ref ALTER CONSTRAINT ref_f1_f2_fkey + DEFERRABLE INITIALLY DEFERRED; +BEGIN; +DELETE FROM pt; +DELETE FROM ref; +ABORT; +DROP TABLE pt, ref; +-- Partitioned table at referenced end +CREATE TABLE pt(f1 int, f2 int, f3 int, PRIMARY KEY(f1,f2)) + PARTITION BY LIST(f1); +CREATE TABLE pt1 PARTITION OF pt FOR VALUES IN (1); +CREATE TABLE pt2 PARTITION OF pt FOR VALUES IN (2); +CREATE TABLE ref(f1 int, f2 int, f3 int); +ALTER TABLE ref ADD FOREIGN KEY(f1,f2) REFERENCES pt; +ALTER TABLE ref ALTER CONSTRAINT ref_f1_f2_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 at referenced end +CREATE TABLE pt(f1 int, f2 int, f3 int, PRIMARY KEY(f1,f2)) + PARTITION BY LIST(f1); +CREATE TABLE pt1_2 PARTITION OF pt FOR VALUES IN (1, 2) PARTITION BY LIST (f1); +CREATE TABLE pt1 PARTITION OF pt1_2 FOR VALUES IN (1); +CREATE TABLE pt2 PARTITION OF pt1_2 FOR VALUES IN (2); +CREATE TABLE ref(f1 int, f2 int, f3 int); +ALTER TABLE ref ADD FOREIGN KEY(f1,f2) REFERENCES pt; +ALTER TABLE ref ALTER CONSTRAINT ref_f1_f2_fkey1 + DEFERRABLE INITIALLY DEFERRED; -- fails +ERROR: cannot alter constraint "ref_f1_f2_fkey1" on relation "ref" +DETAIL: Constraint "ref_f1_f2_fkey1" is derived from constraint "ref_f1_f2_fkey" of relation "ref". +HINT: You may alter the constraint it derives from, instead. +ALTER TABLE ref ALTER CONSTRAINT ref_f1_f2_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; DROP SCHEMA fkpart9 CASCADE; NOTICE: drop cascades to 2 other objects DETAIL: drop cascades to table pk diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index 67aa20435d..de417b62b6 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -1636,6 +1636,81 @@ SET CONSTRAINTS fk_a_fkey DEFERRED; DELETE FROM pk WHERE a = 1; DELETE FROM fk WHERE a = 1; COMMIT; -- OK + +-- 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_f2_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_f2_fkey + DEFERRABLE INITIALLY IMMEDIATE; -- fails +ALTER TABLE ref ALTER CONSTRAINT ref_f1_f2_fkey + DEFERRABLE INITIALLY DEFERRED; +BEGIN; +DELETE FROM pt; +DELETE FROM ref; +ABORT; +DROP TABLE pt, ref; + +-- Partitioned table at referenced end +CREATE TABLE pt(f1 int, f2 int, f3 int, PRIMARY KEY(f1,f2)) + PARTITION BY LIST(f1); +CREATE TABLE pt1 PARTITION OF pt FOR VALUES IN (1); +CREATE TABLE pt2 PARTITION OF pt FOR VALUES IN (2); +CREATE TABLE ref(f1 int, f2 int, f3 int); +ALTER TABLE ref ADD FOREIGN KEY(f1,f2) REFERENCES pt; +ALTER TABLE ref ALTER CONSTRAINT ref_f1_f2_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 at referenced end +CREATE TABLE pt(f1 int, f2 int, f3 int, PRIMARY KEY(f1,f2)) + PARTITION BY LIST(f1); +CREATE TABLE pt1_2 PARTITION OF pt FOR VALUES IN (1, 2) PARTITION BY LIST (f1); +CREATE TABLE pt1 PARTITION OF pt1_2 FOR VALUES IN (1); +CREATE TABLE pt2 PARTITION OF pt1_2 FOR VALUES IN (2); +CREATE TABLE ref(f1 int, f2 int, f3 int); +ALTER TABLE ref ADD FOREIGN KEY(f1,f2) REFERENCES pt; +ALTER TABLE ref ALTER CONSTRAINT ref_f1_f2_fkey1 + DEFERRABLE INITIALLY DEFERRED; -- fails +ALTER TABLE ref ALTER CONSTRAINT ref_f1_f2_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; + DROP SCHEMA fkpart9 CASCADE; -- Verify ON UPDATE/DELETE behavior