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

From 92da65c2359db0bb6947a283a7996273d22a014a Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Wed, 9 Jan 2019 14:01:47 +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 c88e923327..60e4ff5594 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -478,8 +478,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;
@@ -500,8 +498,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)
@@ -702,9 +698,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));
@@ -1156,8 +1149,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)
@@ -1166,8 +1159,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));
@@ -1179,25 +1170,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 d263903622..d8430f6fe5 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -972,9 +972,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 (!cldidx->rd_index->indisvalid)
 							invalidate_parent = true;
 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index a3eb39ffdd..8e2fb9b189 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7248,6 +7248,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
@@ -7586,6 +7589,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,
@@ -7611,9 +7634,9 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 									  NULL, /* no exclusion constraint */
 									  NULL, /* no check constraint */
 									  NULL,
-									  true, /* islocal */
-									  0,	/* inhcount */
-									  true, /* isnoinherit */
+									  conislocal,
+									  coninhcount,
+									  connoinherit,
 									  false);	/* is_internal */
 	ObjectAddressSet(address, ConstraintRelationId, constrOid);
 
@@ -7662,20 +7685,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);
 		}
@@ -8925,9 +8943,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\"",
@@ -14688,6 +14710,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