diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index d9ba87a2a3..496d7da7f3 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -355,8 +355,13 @@ static void AlterIndexNamespaces(Relation classRel, Relation rel, static void AlterSeqNamespaces(Relation classRel, Relation rel, Oid oldNspOid, Oid newNspOid, ObjectAddresses *objsMoved, LOCKMODE lockmode); -static ObjectAddress ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, - bool recurse, bool recursing, LOCKMODE lockmode); +static ObjectAddress ATExecAlterConstraint(List **wqueue, AlteredTableInfo *tab, + 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 +4674,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 */ @@ -4967,7 +4973,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, lockmode); break; case AT_AlterConstraint: /* ALTER CONSTRAINT */ - address = ATExecAlterConstraint(rel, cmd, false, false, lockmode); + address = ATExecAlterConstraint(wqueue, tab, rel, cmd, false, false, lockmode); break; case AT_ValidateConstraint: /* VALIDATE CONSTRAINT */ address = ATExecValidateConstraint(wqueue, rel, cmd->name, false, @@ -10190,28 +10196,30 @@ 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(List **wqueue, AlteredTableInfo *tab, 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 +10253,88 @@ 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 a warning. + * + * The problem is that altering a non-topmost constraint leaves action + * triggers untouched, since they are not directly connected to this + * constraint but rather to its parent. We would like to forbid this + * action but cannot, because existing databases might be in this state + * already and pg_dumps would become unrestorable. + */ + if (OidIsValid(currcon->conparentid)) + ereport(WARNING, + (errmsg("constraint \"%s\" of relation \"%s\" is not top-level -- proceeding anyway", + cmdcon->conname, RelationGetRelationName(rel)), + errhint("You may want to alter the constraint in the ancestor table 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, currcon->oid); + } + + /* + * ATExecConstrRecurse already invalidated relcache for the relations + * having the constraint itself; we also need to invalidate relations that + * have any triggers that implement 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 + * + * *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 we don't do currently 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; - 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,20 +10342,29 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, CatalogTupleUpdate(conrel, ©Tuple->t_self, copyTuple); InvokeObjectPostAlterHook(ConstraintRelationId, - currcon->oid, 0); + conoid, 0); heap_freetuple(copyTuple); + changed = true; + } - /* - * Now we need to update the multiple entries in pg_trigger that - * implement the constraint. - */ - tgrel = table_open(TriggerRelationId, RowExclusiveLock); + /* Make new constraint flags visible to others */ + /* XXX only if the constraint changed? */ + CacheInvalidateRelcache(rel); + + /* + * Now we need to update the multiple entries in pg_trigger that implement + * the constraint. + */ + { + HeapTuple tgtuple; + ScanKeyData tgkey; + SysScanDesc tgscan; ScanKeyInit(&tgkey, Anum_pg_trigger_tgconstraint, BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(currcon->oid)); + ObjectIdGetDatum(conoid)); tgscan = systable_beginscan(tgrel, TriggerConstraintIndexId, true, NULL, 1, &tgkey); @@ -10289,6 +10373,7 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, { 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 +10382,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, @@ -10319,37 +10404,52 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd, copy_tg->tginitdeferred = cmdcon->initdeferred; CatalogTupleUpdate(tgrel, ©Tuple->t_self, copyTuple); - InvokeObjectPostAlterHook(TriggerRelationId, currcon->oid, 0); + InvokeObjectPostAlterHook(TriggerRelationId, tgform->oid, 0); heap_freetuple(copyTuple); } 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; } /* @@ -17764,6 +17864,8 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent, * 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. + * + * XXX is this correct if the partition is partitioned? */ fkconstraint = makeNode(Constraint); fkconstraint->conname = pstrdup(NameStr(conform->conname)); diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index 7386f4d635..efd061b857 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -2313,6 +2313,95 @@ 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); +ALTER TABLE ref ADD FOREIGN KEY(f1,f2) REFERENCES pt; +INSERT INTO pt VALUES(1,2,3), (1,1,3); +INSERT INTO ref VALUES(1,2,3), (1,1,3); +SELECT tableoid::regclass, * from ref; + tableoid | f1 | f2 | f3 +----------+----+----+---- + ref1 | 1 | 1 | 3 + ref2 | 1 | 2 | 3 +(2 rows) + +ALTER TABLE ref1_2 ALTER CONSTRAINT ref_f1_f2_fkey + DEFERRABLE INITIALLY DEFERRED; +WARNING: constraint "ref_f1_f2_fkey" of relation "ref1_2" is not top-level -- proceeding anyway +HINT: You may want to alter the constraint in the ancestor table instead. +ALTER TABLE ref2 ALTER CONSTRAINT ref_f1_f2_fkey + DEFERRABLE INITIALLY IMMEDIATE; +WARNING: constraint "ref_f1_f2_fkey" of relation "ref2" is not top-level -- proceeding anyway +HINT: You may want to alter the constraint in the ancestor table instead. +BEGIN; +DELETE FROM pt; -- fails +ERROR: update or delete on table "pt" violates foreign key constraint "ref_f1_f2_fkey" on table "ref" +DETAIL: Key (f1, f2)=(1, 2) is still referenced from table "ref". +DELETE FROM ref; +ERROR: current transaction is aborted, commands ignored until end of transaction block +ABORT; +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_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..124993263d 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -1636,6 +1636,85 @@ 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); +ALTER TABLE ref ADD FOREIGN KEY(f1,f2) REFERENCES pt; +INSERT INTO pt VALUES(1,2,3), (1,1,3); +INSERT INTO ref VALUES(1,2,3), (1,1,3); +SELECT tableoid::regclass, * from ref; +ALTER TABLE ref1_2 ALTER CONSTRAINT ref_f1_f2_fkey + DEFERRABLE INITIALLY DEFERRED; +ALTER TABLE ref2 ALTER CONSTRAINT ref_f1_f2_fkey + DEFERRABLE INITIALLY IMMEDIATE; +BEGIN; +DELETE FROM pt; -- fails +DELETE FROM ref; +ABORT; +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_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