PG11-v1-0002-Do-not-track-foreign-key-inheritance-by-dependenc.patch

From 669c0621d7970e04663117c43ba877ed53a804df Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Wed, 9 Jan 2019 18:52:16 +0900
Subject: [PATCH v1 2/2] Do not track foreign key inheritance by dependencies

Inheritance information maintained in pg_constraint is enough to
prevent a child constraint to be dropped and for them to be dropped
when the parent constraint is dropped.  So, do not create
dependencies between the parent foreign key constraint and its
children.

Also, fix ATAddForeignKeyConstraint to set inheritance information
correctly.
---
 src/backend/catalog/pg_constraint.c | 33 +++++++--------------
 src/backend/commands/indexcmds.c    | 18 ++++++++++++
 src/backend/commands/tablecmds.c    | 57 ++++++++++++++++++++++++++++---------
 3 files changed, 72 insertions(+), 36 deletions(-)

diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index a379bec202..4f0d40473d 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -482,8 +482,6 @@ clone_fk_constraints(Relation pg_constraint, Relation parentRel,
 		Constraint *fkconstraint;
 		bool		attach_it;
 		Oid			constrOid;
-		ObjectAddress parentAddr,
-					childAddr;
 		int			nelem;
 		ListCell   *cell;
 		int			i;
@@ -504,8 +502,6 @@ clone_fk_constraints(Relation pg_constraint, Relation parentRel,
 			continue;
 		}
 
-		ObjectAddressSet(parentAddr, ConstraintRelationId, parentConstrOid);
-
 		datum = fastgetattr(tuple, Anum_pg_constraint_conkey,
 							tupdesc, &isnull);
 		if (isnull)
@@ -708,9 +704,6 @@ clone_fk_constraints(Relation pg_constraint, Relation parentRel,
 								  1, false, true);
 		subclone = lappend_oid(subclone, constrOid);
 
-		ObjectAddressSet(childAddr, ConstraintRelationId, constrOid);
-		recordDependencyOn(&childAddr, &parentAddr, DEPENDENCY_INTERNAL_AUTO);
-
 		fkconstraint = makeNode(Constraint);
 		/* for now this is all we need */
 		fkconstraint->conname = pstrdup(NameStr(constrForm->conname));
@@ -1162,8 +1155,8 @@ AlterConstraintNamespaces(Oid ownerId, Oid oldNspId,
  * ConstraintSetParentConstraint
  *		Set a partition's constraint as child of its parent table's
  *
- * This updates the constraint's pg_constraint row to show it as inherited, and
- * add a dependency to the parent so that it cannot be removed on its own.
+ * This updates the constraint's pg_constraint row to change its
+ * inheritance properties.
  */
 void
 ConstraintSetParentConstraint(Oid childConstrId, Oid parentConstrId)
@@ -1172,8 +1165,6 @@ ConstraintSetParentConstraint(Oid childConstrId, Oid parentConstrId)
 	Form_pg_constraint constrForm;
 	HeapTuple	tuple,
 				newtup;
-	ObjectAddress depender;
-	ObjectAddress referenced;
 
 	constrRel = heap_open(ConstraintRelationId, RowExclusiveLock);
 	tuple = SearchSysCache1(CONSTROID, ObjectIdGetDatum(childConstrId));
@@ -1185,25 +1176,23 @@ ConstraintSetParentConstraint(Oid childConstrId, Oid parentConstrId)
 	{
 		constrForm->conislocal = false;
 		constrForm->coninhcount++;
+		/*
+		 * An inherited foreign key constraint can never have more than one
+		 * parent, because inheriting foreign keys is only allowed for
+		 * partitioning where multiple inheritance is disallowed.
+		 */
+		Assert(constrForm->coninhcount == 1);
 		constrForm->conparentid = parentConstrId;
 
 		CatalogTupleUpdate(constrRel, &tuple->t_self, newtup);
-
-		ObjectAddressSet(referenced, ConstraintRelationId, parentConstrId);
-		ObjectAddressSet(depender, ConstraintRelationId, childConstrId);
-
-		recordDependencyOn(&depender, &referenced, DEPENDENCY_INTERNAL_AUTO);
 	}
 	else
 	{
 		constrForm->coninhcount--;
-		if (constrForm->coninhcount <= 0)
-			constrForm->conislocal = true;
+		/* See the above comment. */
+		Assert(constrForm->coninhcount == 0);
+		constrForm->conislocal = true;
 		constrForm->conparentid = InvalidOid;
-
-		deleteDependencyRecordsForClass(ConstraintRelationId, childConstrId,
-										ConstraintRelationId,
-										DEPENDENCY_INTERNAL_AUTO);
 		CatalogTupleUpdate(constrRel, &tuple->t_self, newtup);
 	}
 
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 965b9f0d23..9e227796bc 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -974,9 +974,27 @@ DefineIndex(Oid relationId,
 						/* Attach index to parent and we're done. */
 						IndexSetParentIndex(cldidx, indexRelationId);
 						if (createdConstraintId != InvalidOid)
+						{
+							ObjectAddress depender;
+							ObjectAddress referenced;
+
 							ConstraintSetParentConstraint(cldConstrOid,
 														  createdConstraintId);
 
+							/*
+							 * Need to set an explicit dependency in this
+							 * case unlike other types of constraints where
+							 * the child constraint gets dropped due to
+							 * inheritance recursion.
+							 */
+							ObjectAddressSet(referenced, ConstraintRelationId,
+											 createdConstraintId);
+							ObjectAddressSet(depender, ConstraintRelationId,
+											 cldConstrOid);
+							recordDependencyOn(&depender, &referenced,
+											   DEPENDENCY_INTERNAL_AUTO);
+						}
+
 						if (!IndexIsValid(cldidx->rd_index))
 							invalidate_parent = true;
 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index ae83deaf51..1a5da20bf3 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7342,6 +7342,9 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	bool		old_check_ok;
 	ObjectAddress address;
 	ListCell   *old_pfeqop_item = list_head(fkconstraint->old_conpfeqop);
+	bool		conislocal = true;
+	int			coninhcount = 0;
+	bool		connoinherit = true;
 
 	/*
 	 * Grab ShareRowExclusiveLock on the pk table, so that someone doesn't
@@ -7680,6 +7683,26 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	/*
 	 * Record the FK constraint in pg_constraint.
 	 */
+
+	/*
+	 * Set the inheritance related properties correctly if it's being
+	 * recursively added for a partition.
+	 */
+	if (OidIsValid(parentConstr))
+	{
+		/* Foreign keys are inherited only for partitioning. */
+		Assert(rel->rd_rel->relispartition);
+		conislocal = false;
+		/* Partitions can have only one parent. */
+		coninhcount = 1;
+		/* Make sure that the constraint can be further inherited. */
+		connoinherit = false;
+	}
+
+	/* If partitioned table, constraint must be marked as inheritable. */
+	if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+		connoinherit = false;
+
 	constrOid = CreateConstraintEntry(fkconstraint->conname,
 									  RelationGetNamespace(rel),
 									  CONSTRAINT_FOREIGN,
@@ -7706,9 +7729,9 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 									  NULL, /* no check constraint */
 									  NULL,
 									  NULL,
-									  true, /* islocal */
-									  0,	/* inhcount */
-									  true, /* isnoinherit */
+									  conislocal,
+									  coninhcount,
+									  connoinherit,
 									  false);	/* is_internal */
 	ObjectAddressSet(address, ConstraintRelationId, constrOid);
 
@@ -7757,20 +7780,15 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 			Oid			partitionId = partdesc->oids[i];
 			Relation	partition = heap_open(partitionId, lockmode);
 			AlteredTableInfo *childtab;
-			ObjectAddress childAddr;
 
 			CheckTableNotInUse(partition, "ALTER TABLE");
 
 			/* Find or create work queue entry for this table */
 			childtab = ATGetQueueEntry(wqueue, partition);
 
-			childAddr =
-				ATAddForeignKeyConstraint(wqueue, childtab, partition,
-										  fkconstraint, constrOid,
-										  recurse, true, lockmode);
-
-			/* Record this constraint as dependent on the parent one */
-			recordDependencyOn(&childAddr, &address, DEPENDENCY_INTERNAL_AUTO);
+			ATAddForeignKeyConstraint(wqueue, childtab, partition,
+									  fkconstraint, constrOid,
+									  recurse, true, lockmode);
 
 			heap_close(partition, NoLock);
 		}
@@ -9024,9 +9042,13 @@ ATExecDropConstraint(Relation rel, const char *constrName,
 
 		con = (Form_pg_constraint) GETSTRUCT(copy_tuple);
 
-		/* Right now only CHECK constraints can be inherited */
-		if (con->contype != CONSTRAINT_CHECK)
-			elog(ERROR, "inherited constraint is not a CHECK constraint");
+		/*
+		 * Right now only CHECK constraints and foreign key constraints can
+		 * be inherited.
+		 */
+		if (con->contype != CONSTRAINT_CHECK &&
+			con->contype != CONSTRAINT_FOREIGN)
+			elog(ERROR, "inherited constraint is not a CHECK constraint or a foreign key constraint");
 
 		if (con->coninhcount <= 0)	/* shouldn't happen */
 			elog(ERROR, "relation %u has non-inherited constraint \"%s\"",
@@ -14826,6 +14848,13 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
 			elog(ERROR, "cache lookup failed for constraint %u", fk->conoid);
 		conform = (Form_pg_constraint) GETSTRUCT(contup);
 
+		/* No need to touch the partition's own foreign keys. */
+		if (conform->conislocal)
+		{
+			ReleaseSysCache(contup);
+			continue;
+		}
+
 		ConstraintSetParentConstraint(fk->conoid, InvalidOid);
 
 		/*
-- 
2.11.0