From 06959fbc2b03eaef8ec1b85a4d3cc587d37e1c46 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Mon, 3 May 2021 20:20:22 -0400
Subject: [PATCH] fix ALTER CONSTRAINT for partitioned tables

---
 src/backend/commands/tablecmds.c          | 172 +++++++++++++++++-----
 src/test/regress/expected/foreign_key.out |  61 +++++++-
 src/test/regress/sql/foreign_key.sql      |  49 +++++-
 3 files changed, 245 insertions(+), 37 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index a149ca044c..271da62155 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -320,8 +320,12 @@ 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,
+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);
@@ -3953,6 +3957,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 */
@@ -4197,7 +4202,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
 											   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,
@@ -8319,20 +8324,25 @@ CloneFkReferencing(Relation pg_constraint, Relation parentRel,
  * InvalidObjectAddress.
  */
 static ObjectAddress
-ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd,
+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 = heap_open(ConstraintRelationId, RowExclusiveLock);
+	tgrel = heap_open(TriggerRelationId, RowExclusiveLock);
 
 	/*
 	 * Find and check the target constraint
@@ -8366,17 +8376,86 @@ 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,
+							 HeapTupleGetOid(contuple));
+	}
+
+	/*
+	 * 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);
+
+	heap_close(conrel, RowExclusiveLock);
+	heap_close(tgrel, 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;
+	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;
-		ScanKeyData tgkey;
-		SysScanDesc tgscan;
-		Relation	tgrel;
-		ListCell   *lc;
 
 		/*
 		 * Now update the catalog, while we have the door open.
@@ -8391,12 +8470,21 @@ ATExecAlterConstraint(Relation rel, AlterTableCmd *cmd,
 								  HeapTupleGetOid(contuple), 0);
 
 		heap_freetuple(copyTuple);
+		changed = true;
+	}
 
-		/*
-		 * Now we need to update the multiple entries in pg_trigger that
-		 * implement the constraint.
-		 */
-		tgrel = heap_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,
@@ -8410,6 +8498,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.
@@ -8418,8 +8507,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 +8536,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..78e6966fd1 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1954,7 +1954,66 @@ 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);
+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_fkey
+  DEFERRABLE INITIALLY DEFERRED;
+WARNING:  constraint "ref_f1_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_fkey
+  DEFERRABLE INITIALLY IMMEDIATE;
+WARNING:  constraint "ref_f1_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_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_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..81a4fa102b 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -1409,6 +1409,53 @@ 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);
+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_fkey
+  DEFERRABLE INITIALLY DEFERRED;
+ALTER TABLE ref2 ALTER CONSTRAINT ref_f1_fkey
+  DEFERRABLE INITIALLY IMMEDIATE;
+BEGIN;
+DELETE FROM pt;        -- fails
+DELETE FROM ref;
+ABORT;
+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
-- 
2.20.1

