problems with foreign keys on partitioned tables

From 9c877b0cb1b8dbe7f6957b5b1256686f3690dee5 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Fri, 18 Jan 2019 14:13:11 +0900
Subject: [PATCH v2 1/2] Ensure PK-side action triggers for partitions after
 being detached

Detaching a partition from the parent whose foreign key(s) would've
been duplicated in the partition makes the foreign key(s) stop
working corretly.  That's because PK-side action trigger for the
foreign key would refer to the parent as the referencing relation
and after the partition is detached it's data is no longer accessible
via parent.  So while the check triggers that remain even after
being detached takes care of the one side of enforcing the foreign
key of the detached partition, lack of corresponding PK-side action
trigger to check detached partition's data means the other side
doesn't work.

To fix, add the action triggers on the PK relation that point to
the detached partition when detaching.
---
 src/backend/catalog/pg_constraint.c | 74 +++++++++++++++++++++++++++++--------
 src/backend/commands/tablecmds.c    | 42 +++++++++++++++++++--
 src/include/commands/tablecmds.h    |  3 +-
 3 files changed, 98 insertions(+), 21 deletions(-)

diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index f4057a9f15..9df6540ac3 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -26,6 +26,7 @@
 #include "catalog/partition.h"
 #include "catalog/pg_constraint.h"
 #include "catalog/pg_operator.h"
+#include "catalog/pg_trigger.h"
 #include "catalog/pg_type.h"
 #include "commands/defrem.h"
 #include "commands/tablecmds.h"
@@ -551,20 +552,6 @@ clone_fk_constraints(Relation pg_constraint, Relation parentRel,
 			elog(ERROR, "conpfeqop is not a 1-D OID array");
 		memcpy(conpfeqop, ARR_DATA_PTR(arr), nelem * sizeof(Oid));
 
-		datum = fastgetattr(tuple, Anum_pg_constraint_conpfeqop,
-							tupdesc, &isnull);
-		if (isnull)
-			elog(ERROR, "null conpfeqop");
-		arr = DatumGetArrayTypeP(datum);
-		nelem = ARR_DIMS(arr)[0];
-		if (ARR_NDIM(arr) != 1 ||
-			nelem < 1 ||
-			nelem > INDEX_MAX_KEYS ||
-			ARR_HASNULL(arr) ||
-			ARR_ELEMTYPE(arr) != OIDOID)
-			elog(ERROR, "conpfeqop is not a 1-D OID array");
-		memcpy(conpfeqop, ARR_DATA_PTR(arr), nelem * sizeof(Oid));
-
 		datum = fastgetattr(tuple, Anum_pg_constraint_conppeqop,
 							tupdesc, &isnull);
 		if (isnull)
@@ -607,6 +594,10 @@ clone_fk_constraints(Relation pg_constraint, Relation parentRel,
 			ForeignKeyCacheInfo *fk = lfirst_node(ForeignKeyCacheInfo, cell);
 			Form_pg_constraint partConstr;
 			HeapTuple	partcontup;
+			Relation	trigrel;
+			HeapTuple	trigtup;
+			HeapScanDesc scan;
+			ScanKeyData key[3];
 
 			attach_it = true;
 
@@ -658,7 +649,57 @@ clone_fk_constraints(Relation pg_constraint, Relation parentRel,
 
 			ReleaseSysCache(partcontup);
 
-			/* looks good!  Attach this constraint */
+			/*
+			 * looks good!  Attach this constraint.  Drop the action triggers
+			 * on the PK side that refer to this partition rel as part of
+			 * this constraint, because they will be wasteful after attaching
+			 * this constraint to the parent constraint.  After being attached,
+			 * action triggers corresponding to the parent constraint can
+			 * indirectly refer to this partition's data by referencing the
+			 * parent relation.
+			 */
+			trigrel = heap_open(TriggerRelationId, AccessShareLock);
+			ScanKeyInit(&key[0],
+						Anum_pg_trigger_tgrelid,
+						BTEqualStrategyNumber, F_OIDEQ,
+						ObjectIdGetDatum(fk->confrelid));
+			ScanKeyInit(&key[1],
+						Anum_pg_trigger_tgconstrrelid,
+						BTEqualStrategyNumber, F_OIDEQ,
+						ObjectIdGetDatum(fk->conrelid));
+			ScanKeyInit(&key[2],
+						Anum_pg_trigger_tgconstraint,
+						BTEqualStrategyNumber, F_OIDEQ,
+						ObjectIdGetDatum(fk->conoid));
+			scan = heap_beginscan_catalog(trigrel, 3, key);
+
+			/*
+			 * There should be two tuples to be deleted corresponding to
+			 * the two action triggers that createForeignKeyActionTriggers
+			 * would have created.
+			 */
+			trigtup = heap_getnext(scan, ForwardScanDirection);
+			Assert(trigtup != NULL);
+			deleteDependencyRecordsForClass(TriggerRelationId,
+											HeapTupleGetOid(trigtup),
+											ConstraintRelationId,
+											DEPENDENCY_INTERNAL);
+			CatalogTupleDelete(trigrel, &trigtup->t_self);
+
+			trigtup = heap_getnext(scan, ForwardScanDirection);
+			Assert(trigtup != NULL);
+			deleteDependencyRecordsForClass(TriggerRelationId,
+											HeapTupleGetOid(trigtup),
+											ConstraintRelationId,
+											DEPENDENCY_INTERNAL);
+			CatalogTupleDelete(trigrel, &trigtup->t_self);
+
+			trigtup = heap_getnext(scan, ForwardScanDirection);
+			Assert(trigtup == NULL);
+
+			heap_endscan(scan);
+			heap_close(trigrel, AccessShareLock);
+
 			ConstraintSetParentConstraint(fk->conoid,
 										  HeapTupleGetOid(tuple));
 			CommandCounterIncrement();
@@ -720,7 +761,8 @@ clone_fk_constraints(Relation pg_constraint, Relation parentRel,
 		fkconstraint->initdeferred = constrForm->condeferred;
 
 		createForeignKeyTriggers(partRel, constrForm->confrelid, fkconstraint,
-								 constrOid, constrForm->conindid, false);
+								 constrOid, constrForm->conindid, false,
+								 true);
 
 		if (cloned)
 		{
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 71b2e3f134..d668a57ac2 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7718,7 +7718,7 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	 * though the constraints also exist below.
 	 */
 	createForeignKeyTriggers(rel, RelationGetRelid(pkrel), fkconstraint,
-							 constrOid, indexOid, !recursing);
+							 constrOid, indexOid, !recursing, true);
 
 	/*
 	 * Tell Phase 3 to check that the constraint is satisfied by existing
@@ -8827,7 +8827,8 @@ createForeignKeyCheckTriggers(Oid myRelOid, Oid refRelOid,
  */
 void
 createForeignKeyTriggers(Relation rel, Oid refRelOid, Constraint *fkconstraint,
-						 Oid constraintOid, Oid indexOid, bool create_action)
+						 Oid constraintOid, Oid indexOid, bool create_action,
+						 bool create_check)
 {
 	/*
 	 * For the referenced side, create action triggers, if requested.  (If the
@@ -8843,7 +8844,7 @@ createForeignKeyTriggers(Relation rel, Oid refRelOid, Constraint *fkconstraint,
 	 * For the referencing side, create the check triggers.  We only need
 	 * these on the partitions.
 	 */
-	if (rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
+	if (rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE && create_check)
 		createForeignKeyCheckTriggers(RelationGetRelid(rel), refRelOid,
 									  fkconstraint, constraintOid, indexOid);
 
@@ -14887,19 +14888,52 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
 	}
 	heap_close(classRel, RowExclusiveLock);
 
-	/* Detach foreign keys */
+	/* Detach any foreign keys that are inherited */
 	fks = copyObject(RelationGetFKeyList(partRel));
 	foreach(cell, fks)
 	{
 		ForeignKeyCacheInfo *fk = lfirst(cell);
 		HeapTuple	contup;
+		Form_pg_constraint conform;
+		Constraint *fkconstraint;
 
 		contup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(fk->conoid));
 		if (!contup)
 			elog(ERROR, "cache lookup failed for constraint %u", fk->conoid);
+		conform = (Form_pg_constraint) GETSTRUCT(contup);
 
+		/* consider only the inherited foreign keys */
+		if (conform->contype != CONSTRAINT_FOREIGN ||
+			!OidIsValid(conform->conparentid))
+		{
+			ReleaseSysCache(contup);
+			continue;
+		}
+
+		/* unset conparentid and adjust conislocal, coninhcount, etc. */
 		ConstraintSetParentConstraint(fk->conoid, InvalidOid);
 
+		/*
+		 * We'll need to make the action triggers on primary key relation that
+		 * point to this relation as the FK relation.  We need to do this,
+		 * because no PK-side triggers exist for an inherited FK constraint.
+		 * Now that we've detached it from the parent, we'd our own.
+		 */
+		fkconstraint = makeNode(Constraint);
+		fkconstraint->conname = pstrdup(NameStr(conform->conname));
+		fkconstraint->fk_upd_action = conform->confupdtype;
+		fkconstraint->fk_del_action = conform->confdeltype;
+		fkconstraint->deferrable = conform->condeferrable;
+		fkconstraint->initdeferred = conform->condeferred;
+
+		/*
+		 * As we already got the check triggers, no need to recreate them,
+		 * so pass false for create_check.
+		 */
+		createForeignKeyTriggers(partRel, conform->confrelid, fkconstraint,
+								 fk->conoid, conform->conindid,
+								 true, false);
+
 		ReleaseSysCache(contup);
 	}
 	list_free_deep(fks);
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index 138de84e83..dd985a80b6 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -77,7 +77,8 @@ extern void check_of_type(HeapTuple typetuple);
 
 extern void createForeignKeyTriggers(Relation rel, Oid refRelOid,
 						 Constraint *fkconstraint, Oid constraintOid,
-						 Oid indexOid, bool create_action);
+						 Oid indexOid, bool create_action,
+						 bool create_check);
 
 extern void register_on_commit_action(Oid relid, OnCommitAction action);
 extern void remove_on_commit_action(Oid relid);
-- 
2.11.0

From 61c03e3435eb10304361ca093be7687f56463b9f Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Fri, 18 Jan 2019 14:20:13 +0900
Subject: [PATCH v2 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    | 17 +++++++++++++
 src/backend/commands/tablecmds.c    | 50 ++++++++++++++++++++++++++-----------
 3 files changed, 64 insertions(+), 36 deletions(-)

diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index 9df6540ac3..0048b3a436 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -483,8 +483,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;
@@ -505,8 +503,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)
@@ -749,9 +745,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));
@@ -1203,8 +1196,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)
@@ -1213,8 +1206,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));
@@ -1226,25 +1217,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 fec5bc5dd6..1b497b5bd8 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -974,8 +974,25 @@ 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 d668a57ac2..d045b4eddb 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\"",
-- 
2.11.0

From d168bfa988bb8923fb138ceb4026ac005a7f580a Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Wed, 9 Jan 2019 10:00:11 +0900
Subject: [PATCH v2 1/2] Ensure PK-side action triggers for partitions after
 being detached

Detaching a partition from the parent whose foreign key(s) would've
been duplicated in the partition makes the foreign key(s) stop
working corretly.  That's because PK-side action trigger for the
foreign key would refer to the parent as the referencing relation
and after the partition is detached it's data is no longer accessible
via parent.  So while the check triggers that remain even after
being detached takes care of the one side of enforcing the foreign
key of the detached partition, lack of corresponding PK-side action
trigger to check detached partition's data means the other side
doesn't work.

To fix, add the action triggers on the PK relation that point to
the detached partition when detaching.
---
 src/backend/catalog/pg_constraint.c | 74 +++++++++++++++++++++++++++++--------
 src/backend/commands/tablecmds.c    | 42 +++++++++++++++++++--
 src/include/commands/tablecmds.h    |  3 +-
 3 files changed, 98 insertions(+), 21 deletions(-)

diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index 3c960c9423..3cac851447 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -27,6 +27,7 @@
 #include "catalog/partition.h"
 #include "catalog/pg_constraint.h"
 #include "catalog/pg_operator.h"
+#include "catalog/pg_trigger.h"
 #include "catalog/pg_type.h"
 #include "commands/defrem.h"
 #include "commands/tablecmds.h"
@@ -547,20 +548,6 @@ clone_fk_constraints(Relation pg_constraint, Relation parentRel,
 			elog(ERROR, "conpfeqop is not a 1-D OID array");
 		memcpy(conpfeqop, ARR_DATA_PTR(arr), nelem * sizeof(Oid));
 
-		datum = fastgetattr(tuple, Anum_pg_constraint_conpfeqop,
-							tupdesc, &isnull);
-		if (isnull)
-			elog(ERROR, "null conpfeqop");
-		arr = DatumGetArrayTypeP(datum);
-		nelem = ARR_DIMS(arr)[0];
-		if (ARR_NDIM(arr) != 1 ||
-			nelem < 1 ||
-			nelem > INDEX_MAX_KEYS ||
-			ARR_HASNULL(arr) ||
-			ARR_ELEMTYPE(arr) != OIDOID)
-			elog(ERROR, "conpfeqop is not a 1-D OID array");
-		memcpy(conpfeqop, ARR_DATA_PTR(arr), nelem * sizeof(Oid));
-
 		datum = fastgetattr(tuple, Anum_pg_constraint_conppeqop,
 							tupdesc, &isnull);
 		if (isnull)
@@ -603,6 +590,11 @@ clone_fk_constraints(Relation pg_constraint, Relation parentRel,
 			ForeignKeyCacheInfo *fk = lfirst_node(ForeignKeyCacheInfo, cell);
 			Form_pg_constraint partConstr;
 			HeapTuple	partcontup;
+			Relation	trigrel;
+			HeapTuple	trigtup;
+			Form_pg_trigger trigform;
+			HeapScanDesc scan;
+			ScanKeyData key[3];
 
 			attach_it = true;
 
@@ -654,7 +646,56 @@ clone_fk_constraints(Relation pg_constraint, Relation parentRel,
 
 			ReleaseSysCache(partcontup);
 
-			/* looks good!  Attach this constraint */
+			/*
+			 * looks good!  Attach this constraint.  Drop the action triggers
+			 * on the PK side that refer to this partition rel as part of
+			 * this constraint, because they will be wasteful after attaching
+			 * this constraint to the parent constraint.  After being attached,
+			 * action triggers corresponding to the parent constraint can
+			 * indirectly refer to this partition's data by referencing the
+			 * parent relation.
+			 */
+			trigrel = heap_open(TriggerRelationId, AccessShareLock);
+			ScanKeyInit(&key[0],
+						Anum_pg_trigger_tgrelid,
+						BTEqualStrategyNumber, F_OIDEQ,
+						ObjectIdGetDatum(fk->confrelid));
+			ScanKeyInit(&key[1],
+						Anum_pg_trigger_tgconstrrelid,
+						BTEqualStrategyNumber, F_OIDEQ,
+						ObjectIdGetDatum(fk->conrelid));
+			ScanKeyInit(&key[2],
+						Anum_pg_trigger_tgconstraint,
+						BTEqualStrategyNumber, F_OIDEQ,
+						ObjectIdGetDatum(fk->conoid));
+			scan = heap_beginscan_catalog(trigrel, 3, key);
+
+			/*
+			 * There should be two tuples to be deleted corresponding to
+			 * the two action triggers that createForeignKeyActionTriggers
+			 * would have created.
+			 */
+			trigtup = heap_getnext(scan, ForwardScanDirection);
+			Assert(trigtup != NULL);
+			trigform = (Form_pg_trigger) GETSTRUCT(trigtup);
+			deleteDependencyRecordsForClass(TriggerRelationId, trigform->oid,
+											ConstraintRelationId,
+											DEPENDENCY_INTERNAL);
+			CatalogTupleDelete(trigrel, &trigtup->t_self);
+
+			trigtup = heap_getnext(scan, ForwardScanDirection);
+			Assert(trigtup != NULL);
+			trigform = (Form_pg_trigger) GETSTRUCT(trigtup);
+			deleteDependencyRecordsForClass(TriggerRelationId, trigform->oid,
+											ConstraintRelationId,
+											DEPENDENCY_INTERNAL);
+			CatalogTupleDelete(trigrel, &trigtup->t_self);
+
+			trigtup = heap_getnext(scan, ForwardScanDirection);
+			Assert(trigtup == NULL);
+			heap_endscan(scan);
+			heap_close(trigrel, AccessShareLock);
+
 			ConstraintSetParentConstraint(fk->conoid, constrForm->oid);
 			CommandCounterIncrement();
 			attach_it = true;
@@ -714,7 +755,8 @@ clone_fk_constraints(Relation pg_constraint, Relation parentRel,
 		fkconstraint->initdeferred = constrForm->condeferred;
 
 		createForeignKeyTriggers(partRel, constrForm->confrelid, fkconstraint,
-								 constrOid, constrForm->conindid, false);
+								 constrOid, constrForm->conindid, false,
+								 true);
 
 		if (cloned)
 		{
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d2781cbf19..80b46d3139 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7623,7 +7623,7 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	 * though the constraints also exist below.
 	 */
 	createForeignKeyTriggers(rel, RelationGetRelid(pkrel), fkconstraint,
-							 constrOid, indexOid, !recursing);
+							 constrOid, indexOid, !recursing, true);
 
 	/*
 	 * Tell Phase 3 to check that the constraint is satisfied by existing
@@ -8728,7 +8728,8 @@ createForeignKeyCheckTriggers(Oid myRelOid, Oid refRelOid,
  */
 void
 createForeignKeyTriggers(Relation rel, Oid refRelOid, Constraint *fkconstraint,
-						 Oid constraintOid, Oid indexOid, bool create_action)
+						 Oid constraintOid, Oid indexOid, bool create_action,
+						 bool create_check)
 {
 	/*
 	 * For the referenced side, create action triggers, if requested.  (If the
@@ -8744,7 +8745,7 @@ createForeignKeyTriggers(Relation rel, Oid refRelOid, Constraint *fkconstraint,
 	 * For the referencing side, create the check triggers.  We only need
 	 * these on the partitions.
 	 */
-	if (rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
+	if (rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE && create_check)
 		createForeignKeyCheckTriggers(RelationGetRelid(rel), refRelOid,
 									  fkconstraint, constraintOid, indexOid);
 
@@ -14749,19 +14750,52 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
 	}
 	heap_close(classRel, RowExclusiveLock);
 
-	/* Detach foreign keys */
+	/* Detach any foreign keys that are inherited */
 	fks = copyObject(RelationGetFKeyList(partRel));
 	foreach(cell, fks)
 	{
 		ForeignKeyCacheInfo *fk = lfirst(cell);
 		HeapTuple	contup;
+		Form_pg_constraint conform;
+		Constraint *fkconstraint;
 
 		contup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(fk->conoid));
 		if (!contup)
 			elog(ERROR, "cache lookup failed for constraint %u", fk->conoid);
+		conform = (Form_pg_constraint) GETSTRUCT(contup);
 
+		/* consider only the inherited foreign keys */
+		if (conform->contype != CONSTRAINT_FOREIGN ||
+			!OidIsValid(conform->conparentid))
+		{
+			ReleaseSysCache(contup);
+			continue;
+		}
+
+		/* unset conparentid and adjust conislocal, coninhcount, etc. */
 		ConstraintSetParentConstraint(fk->conoid, InvalidOid);
 
+		/*
+		 * We'll need to make the action triggers on primary key relation that
+		 * point to this relation as the FK relation.  We need to do this,
+		 * because no PK-side triggers exist for an inherited FK constraint.
+		 * Now that we've detached it from the parent, we'd our own.
+		 */
+		fkconstraint = makeNode(Constraint);
+		fkconstraint->conname = pstrdup(NameStr(conform->conname));
+		fkconstraint->fk_upd_action = conform->confupdtype;
+		fkconstraint->fk_del_action = conform->confdeltype;
+		fkconstraint->deferrable = conform->condeferrable;
+		fkconstraint->initdeferred = conform->condeferred;
+
+		/*
+		 * As we already got the check triggers, no need to recreate them,
+		 * so pass false for create_check.
+		 */
+		createForeignKeyTriggers(partRel, conform->confrelid, fkconstraint,
+								 fk->conoid, conform->conindid,
+								 true, false);
+
 		ReleaseSysCache(contup);
 	}
 	list_free_deep(fks);
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index ec3bb90b01..6bebc68425 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -78,7 +78,8 @@ extern void check_of_type(HeapTuple typetuple);
 
 extern void createForeignKeyTriggers(Relation rel, Oid refRelOid,
 						 Constraint *fkconstraint, Oid constraintOid,
-						 Oid indexOid, bool create_action);
+						 Oid indexOid, bool create_action,
+						 bool create_check);
 
 extern void register_on_commit_action(Oid relid, OnCommitAction action);
 extern void remove_on_commit_action(Oid relid);
-- 
2.11.0

From 3c2ec8fb45df5b807cb00fa9d2c742451372bda2 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Wed, 9 Jan 2019 14:01:47 +0900
Subject: [PATCH v2 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    | 50 ++++++++++++++++++++++++++-----------
 3 files changed, 65 insertions(+), 36 deletions(-)

diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index 3cac851447..3571416c3d 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -479,8 +479,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;
@@ -501,8 +499,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)
@@ -743,9 +739,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));
@@ -1197,8 +1190,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)
@@ -1207,8 +1200,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));
@@ -1220,25 +1211,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 1959e8a82e..91e0b968ac 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -973,9 +973,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 80b46d3139..96aef0699c 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\"",
-- 
2.11.0

From 2047f054facf3ffaff31e36591279b9bd76ca53c Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Fri, 18 Jan 2019 19:06:40 -0300
Subject: [PATCH 1/2] Add missing relcache reset

Adding a foreign key to a partitioned table requires applying a relcache
reset to each partition, so that the newly added FK is correctly
recorded in its entry on next rebuild.

With the previous coding of ATAddForeignKeyConstraint, this may not have
been necessary, but with the one after 0325d7a5957b, it is.

Noted while testing another bugfix.
---
 src/backend/commands/tablecmds.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1a1ac698e5..5a27f64aa7 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7700,6 +7700,12 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 
 			childtab->constraints = lappend(childtab->constraints, newcon);
 
+			/*
+			 * Also invalidate the partition's relcache entry, so that its
+			 * FK list gets updated on next rebuild.
+			 */
+			CacheInvalidateRelcache(partition);
+
 			heap_close(partition, lockmode);
 		}
 	}
-- 
2.11.0

From 42b96db12aafea22285893e1d96c63a7f684abfd Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Fri, 18 Jan 2019 19:10:29 -0300
Subject: [PATCH 2/2] Fix action triggers for FK constraints on detached
 partitions

---
 src/backend/commands/tablecmds.c     | 72 +++++++++++++++++++++++++++++++++++-
 src/test/regress/sql/foreign_key.sql | 20 +++++++++-
 2 files changed, 89 insertions(+), 3 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 5a27f64aa7..02a55ed3d6 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7858,6 +7858,10 @@ CloneFkReferencing(Relation pg_constraint, Relation parentRel,
 			ForeignKeyCacheInfo *fk = lfirst_node(ForeignKeyCacheInfo, cell);
 			Form_pg_constraint partConstr;
 			HeapTuple	partcontup;
+			Relation	trigrel;
+			HeapTuple	trigtup;
+			SysScanDesc scan;
+			ScanKeyData key;
 
 			attach_it = true;
 
@@ -7909,7 +7913,39 @@ CloneFkReferencing(Relation pg_constraint, Relation parentRel,
 
 			ReleaseSysCache(partcontup);
 
-			/* looks good!  Attach this constraint */
+			/*
+			 * Looks good!  Attach this constraint.  Note that the action
+			 * triggers are no longer needed, so remove them.  We identify
+			 * them because they have our constraint OID, as well as being
+			 * on the referenced rel.
+			 */
+			trigrel = heap_open(TriggerRelationId, RowExclusiveLock);
+			ScanKeyInit(&key,
+						Anum_pg_trigger_tgconstraint,
+						BTEqualStrategyNumber, F_OIDEQ,
+						ObjectIdGetDatum(fk->conoid));
+
+			scan = systable_beginscan(trigrel, TriggerConstraintIndexId, true,
+									  NULL, 1, &key);
+			while ((trigtup = systable_getnext(scan)) != NULL)
+			{
+				Form_pg_trigger	trgform = (Form_pg_trigger) GETSTRUCT(trigtup);
+
+				if (trgform->tgconstrrelid != fk->conrelid)
+					continue;
+				if (trgform->tgrelid != fk->confrelid)
+					continue;
+
+				deleteDependencyRecordsForClass(TriggerRelationId,
+												trgform->oid,
+												ConstraintRelationId,
+												DEPENDENCY_INTERNAL);
+				CatalogTupleDelete(trigrel, &trigtup->t_self);
+			}
+
+			systable_endscan(scan);
+			heap_close(trigrel, RowExclusiveLock);
+
 			ConstraintSetParentConstraint(fk->conoid, parentConstrOid);
 			CommandCounterIncrement();
 			attach_it = true;
@@ -15080,19 +15116,51 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
 	}
 	heap_close(classRel, RowExclusiveLock);
 
-	/* Detach foreign keys */
+	/*
+	 * Detach any foreign keys that are inherited.  This includes creating
+	 * additional action triggers.
+	 */
 	fks = copyObject(RelationGetFKeyList(partRel));
 	foreach(cell, fks)
 	{
 		ForeignKeyCacheInfo *fk = lfirst(cell);
 		HeapTuple	contup;
+		Form_pg_constraint conform;
+		Constraint *fkconstraint;
 
 		contup = SearchSysCache1(CONSTROID, ObjectIdGetDatum(fk->conoid));
 		if (!contup)
 			elog(ERROR, "cache lookup failed for constraint %u", fk->conoid);
+		conform = (Form_pg_constraint) GETSTRUCT(contup);
 
+		/* consider only the inherited foreign keys */
+		if (conform->contype != CONSTRAINT_FOREIGN ||
+			!OidIsValid(conform->conparentid))
+		{
+			ReleaseSysCache(contup);
+			continue;
+		}
+
+		/* unset conparentid and adjust conislocal, coninhcount, etc. */
 		ConstraintSetParentConstraint(fk->conoid, InvalidOid);
 
+		/*
+		 * 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.
+		 */
+		fkconstraint = makeNode(Constraint);
+		fkconstraint->conname = pstrdup(NameStr(conform->conname));
+		fkconstraint->fk_upd_action = conform->confupdtype;
+		fkconstraint->fk_del_action = conform->confdeltype;
+		fkconstraint->deferrable = conform->condeferrable;
+		fkconstraint->initdeferred = conform->condeferred;
+
+		createForeignKeyActionTriggers(partRel, conform->confrelid,
+									   fkconstraint, fk->conoid,
+									   conform->conindid);
+		CommandCounterIncrement();
+
 		ReleaseSysCache(contup);
 	}
 	list_free_deep(fks);
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index c3a7cfcc01..e08b6a21b0 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -1375,6 +1375,24 @@ create table fkpart0.fk_part_56_5 partition of fkpart0.fk_part_56
 alter table fkpart0.fk_part_56 drop constraint fk_part_a_fkey;
 alter table fkpart0.fk_part_56_5 drop constraint fk_part_a_fkey;
 
+-- verify that attaching and detaching partitions maintains the right set of
+-- triggers
+create schema fkpart1;
+set search_path to fkpart1;
+  create table pkey (a int primary key);
+  create table fk_part (a int) partition by list (a);
+  create table fk_part_1 partition of fk_part for values in (1) partition by list (a);
+  create table fk_part_1_1 partition of fk_part_1 for values in (1);
+alter table fkpart1.fk_part add foreign key (a) references fkpart1.pkey;
+insert into fkpart1.fk_part values (1);		-- should fail
+insert into fkpart1.pkey values (1);
+insert into fkpart1.fk_part values (1);
+delete from fkpart1.pkey where a = 1;		-- should fail
+alter table fkpart1.fk_part detach partition fkpart1.fk_part_1;
+create table fkpart1.fk_part_1_2 partition of fkpart1.fk_part_1 for values in (2);
+insert into fkpart1.fk_part_1 values (2);	-- should fail
+delete from fkpart1.pkey where a = 1;
+
 \set VERBOSITY terse	\\ -- suppress cascade details
-drop schema fkpart0 cascade;
+drop schema fkpart0, fkpart1 cascade;
 \set VERBOSITY default
-- 
2.11.0

From 432c4551990d0da1c77b6b9523296b0a2a0a5119 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Tue, 22 Jan 2019 13:20:54 +0900
Subject: [PATCH] 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 connoniherit on the parent
constraint correctly.
---
 src/backend/catalog/pg_constraint.c       | 27 +++++++++++++--------------
 src/backend/commands/indexcmds.c          | 22 ++++++++++++++++++++--
 src/backend/commands/tablecmds.c          | 24 ++++++++++++------------
 src/test/regress/expected/foreign_key.out | 17 +++++++++++++++--
 src/test/regress/sql/foreign_key.sql      | 14 +++++++++++++-
 5 files changed, 73 insertions(+), 31 deletions(-)

diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index 446b54b9ff..855d57c65a 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -762,8 +762,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)
@@ -772,8 +772,6 @@ ConstraintSetParentConstraint(Oid childConstrId, Oid parentConstrId)
 	Form_pg_constraint constrForm;
 	HeapTuple	tuple,
 				newtup;
-	ObjectAddress depender;
-	ObjectAddress referenced;
 
 	constrRel = table_open(ConstraintRelationId, RowExclusiveLock);
 	tuple = SearchSysCache1(CONSTROID, ObjectIdGetDatum(childConstrId));
@@ -785,25 +783,26 @@ 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 3edc94308e..6c8aa5d149 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -972,8 +972,26 @@ DefineIndex(Oid relationId,
 						/* Attach index to parent and we're done. */
 						IndexSetParentIndex(cldidx, indexRelationId);
 						if (createdConstraintId != InvalidOid)
-							ConstraintSetParentConstraint(cldConstrOid,
-														  createdConstraintId);
+						{
+							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 fbd2d101c1..0040ac63d2 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7251,6 +7251,9 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	bool		old_check_ok;
 	ObjectAddress address;
 	ListCell   *old_pfeqop_item = list_head(fkconstraint->old_conpfeqop);
+	/* Partitioned table's foreign key must always be inheritable. */
+	bool		connoinherit = (rel->rd_rel->relkind !=
+								RELKIND_PARTITIONED_TABLE);
 
 	/*
 	 * Grab ShareRowExclusiveLock on the pk table, so that someone doesn't
@@ -7616,7 +7619,7 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 									  NULL,
 									  true, /* islocal */
 									  0,	/* inhcount */
-									  true, /* isnoinherit */
+									  connoinherit,
 									  false);	/* is_internal */
 	ObjectAddressSet(address, ConstraintRelationId, constrOid);
 
@@ -7812,8 +7815,6 @@ CloneFkReferencing(Relation pg_constraint, Relation parentRel,
 		Constraint *fkconstraint;
 		bool		attach_it;
 		Oid			constrOid;
-		ObjectAddress parentAddr,
-					childAddr;
 		ListCell   *cell;
 		int			i;
 
@@ -7830,8 +7831,6 @@ CloneFkReferencing(Relation pg_constraint, Relation parentRel,
 			continue;
 		}
 
-		ObjectAddressSet(parentAddr, ConstraintRelationId, parentConstrOid);
-
 		DeconstructFkConstraintRow(tuple, &numfks, conkey, confkey,
 								   conpfeqop, conppeqop, conffeqop);
 		for (i = 0; i < numfks; i++)
@@ -7986,9 +7985,6 @@ CloneFkReferencing(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));
@@ -9284,11 +9280,15 @@ 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");
+		/*
+		 * 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 */
+		if (con->coninhcount == 0)	/* shouldn't happen */
 			elog(ERROR, "relation %u has non-inherited constraint \"%s\"",
 				 childrelid, constrName);
 
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 76040a7601..ba83c9d9bb 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1945,7 +1945,20 @@ DETAIL:  Key (a)=(2) is not present in table "pkey".
 delete from fkpart1.pkey where a = 1;
 ERROR:  update or delete on table "pkey" violates foreign key constraint "fk_part_a_fkey" on table "fk_part_1"
 DETAIL:  Key (a)=(1) is still referenced from table "fk_part_1".
+-- verify that attaching and detaching partitions manipulates the inheritance
+-- properties of their FK constraints correctly
+create schema fkpart2;
+create table fkpart2.pkey (a int primary key);
+create table fkpart2.fk_part (a int, constraint p_fkey foreign key (a) references fkpart2.pkey) partition by list (a);
+create table fkpart2.fk_part_1 partition of fkpart2.fk_part for values in (1) partition by list (a);
+create table fkpart2.fk_part_1_1 partition of fkpart2.fk_part_1 for values in (1);
+alter table fkpart2.fk_part_1 drop constraint p_fkey;	-- should fail
+ERROR:  cannot drop inherited constraint "p_fkey" of relation "fk_part_1"
+alter table fkpart2.fk_part_1_1 drop constraint p_fkey;	-- should fail
+ERROR:  cannot drop inherited constraint "p_fkey" of relation "fk_part_1_1"
+alter table fkpart2.fk_part detach partition fkpart2.fk_part_1;
+alter table fkpart2.fk_part_1 drop constraint p_fkey;
 \set VERBOSITY terse	\\ -- suppress cascade details
-drop schema fkpart0, fkpart1 cascade;
-NOTICE:  drop cascades to 5 other objects
+drop schema fkpart0, fkpart1, fkpart2 cascade;
+NOTICE:  drop cascades to 8 other objects
 \set VERBOSITY default
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 9ed1166c66..966162d045 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -1392,6 +1392,18 @@ create table fkpart1.fk_part_1_2 partition of fkpart1.fk_part_1 for values in (2
 insert into fkpart1.fk_part_1 values (2);	-- should fail
 delete from fkpart1.pkey where a = 1;
 
+-- verify that attaching and detaching partitions manipulates the inheritance
+-- properties of their FK constraints correctly
+create schema fkpart2;
+create table fkpart2.pkey (a int primary key);
+create table fkpart2.fk_part (a int, constraint p_fkey foreign key (a) references fkpart2.pkey) partition by list (a);
+create table fkpart2.fk_part_1 partition of fkpart2.fk_part for values in (1) partition by list (a);
+create table fkpart2.fk_part_1_1 partition of fkpart2.fk_part_1 for values in (1);
+alter table fkpart2.fk_part_1 drop constraint p_fkey;	-- should fail
+alter table fkpart2.fk_part_1_1 drop constraint p_fkey;	-- should fail
+alter table fkpart2.fk_part detach partition fkpart2.fk_part_1;
+alter table fkpart2.fk_part_1 drop constraint p_fkey;
+
 \set VERBOSITY terse	\\ -- suppress cascade details
-drop schema fkpart0, fkpart1 cascade;
+drop schema fkpart0, fkpart1, fkpart2 cascade;
 \set VERBOSITY default
-- 
2.11.0

From 5178942bc853b3c70ff03750477ae3519d82883f Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Tue, 22 Jan 2019 12:48:12 +0900
Subject: [PATCH] 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 connoniherit on the parent
constraint correctly.
---
 src/backend/catalog/pg_constraint.c       | 27 +++++++++++++--------------
 src/backend/commands/indexcmds.c          | 18 ++++++++++++++++++
 src/backend/commands/tablecmds.c          | 24 ++++++++++++------------
 src/test/regress/expected/foreign_key.out | 17 +++++++++++++++--
 src/test/regress/sql/foreign_key.sql      | 14 +++++++++++++-
 5 files changed, 71 insertions(+), 29 deletions(-)

diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index 5720c652b2..3e939e014f 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -771,8 +771,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)
@@ -781,8 +781,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));
@@ -794,25 +792,26 @@ 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 fec5bc5dd6..048fabcc61 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 5a62adbafc..9ec27eb6bf 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7344,6 +7344,9 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	bool		old_check_ok;
 	ObjectAddress address;
 	ListCell   *old_pfeqop_item = list_head(fkconstraint->old_conpfeqop);
+	/* Partitioned table's foreign key must always be inheritable. */
+	bool		connoinherit = (rel->rd_rel->relkind !=
+								RELKIND_PARTITIONED_TABLE);
 
 	/*
 	 * Grab ShareRowExclusiveLock on the pk table, so that someone doesn't
@@ -7710,7 +7713,7 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 									  NULL,
 									  true, /* islocal */
 									  0,	/* inhcount */
-									  true, /* isnoinherit */
+									  connoinherit,
 									  false);	/* is_internal */
 	ObjectAddressSet(address, ConstraintRelationId, constrOid);
 
@@ -7906,8 +7909,6 @@ CloneFkReferencing(Relation pg_constraint, Relation parentRel,
 		Constraint *fkconstraint;
 		bool		attach_it;
 		Oid			constrOid;
-		ObjectAddress parentAddr,
-					childAddr;
 		ListCell   *cell;
 		int			i;
 
@@ -7924,8 +7925,6 @@ CloneFkReferencing(Relation pg_constraint, Relation parentRel,
 			continue;
 		}
 
-		ObjectAddressSet(parentAddr, ConstraintRelationId, parentConstrOid);
-
 		DeconstructFkConstraintRow(tuple, &numfks, conkey, confkey,
 								   conpfeqop, conppeqop, conffeqop);
 		for (i = 0; i < numfks; i++)
@@ -8081,9 +8080,6 @@ CloneFkReferencing(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));
@@ -9383,11 +9379,15 @@ 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");
+		/*
+		 * 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 */
+		if (con->coninhcount == 0)	/* shouldn't happen */
 			elog(ERROR, "relation %u has non-inherited constraint \"%s\"",
 				 childrelid, constrName);
 
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 4fd2341388..aa44daf0e8 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1884,7 +1884,20 @@ DETAIL:  Key (a)=(2) is not present in table "pkey".
 delete from fkpart1.pkey where a = 1;
 ERROR:  update or delete on table "pkey" violates foreign key constraint "fk_part_a_fkey" on table "fk_part_1"
 DETAIL:  Key (a)=(1) is still referenced from table "fk_part_1".
+-- verify that attaching and detaching partitions manipulates the inheritance
+-- properties of their FK constraints correctly
+create schema fkpart2;
+create table fkpart2.pkey (a int primary key);
+create table fkpart2.fk_part (a int, constraint p_fkey foreign key (a) references fkpart2.pkey) partition by list (a);
+create table fkpart2.fk_part_1 partition of fkpart2.fk_part for values in (1) partition by list (a);
+create table fkpart2.fk_part_1_1 partition of fkpart2.fk_part_1 for values in (1);
+alter table fkpart2.fk_part_1 drop constraint p_fkey;	-- should fail
+ERROR:  cannot drop inherited constraint "p_fkey" of relation "fk_part_1"
+alter table fkpart2.fk_part_1_1 drop constraint p_fkey;	-- should fail
+ERROR:  cannot drop inherited constraint "p_fkey" of relation "fk_part_1_1"
+alter table fkpart2.fk_part detach partition fkpart2.fk_part_1;
+alter table fkpart2.fk_part_1 drop constraint p_fkey;
 \set VERBOSITY terse	\\ -- suppress cascade details
-drop schema fkpart0, fkpart1 cascade;
-NOTICE:  drop cascades to 5 other objects
+drop schema fkpart0, fkpart1, fkpart2 cascade;
+NOTICE:  drop cascades to 8 other objects
 \set VERBOSITY default
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index dd0be01c77..fe9b300d90 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -1341,6 +1341,18 @@ create table fkpart1.fk_part_1_2 partition of fkpart1.fk_part_1 for values in (2
 insert into fkpart1.fk_part_1 values (2);	-- should fail
 delete from fkpart1.pkey where a = 1;
 
+-- verify that attaching and detaching partitions manipulates the inheritance
+-- properties of their FK constraints correctly
+create schema fkpart2;
+create table fkpart2.pkey (a int primary key);
+create table fkpart2.fk_part (a int, constraint p_fkey foreign key (a) references fkpart2.pkey) partition by list (a);
+create table fkpart2.fk_part_1 partition of fkpart2.fk_part for values in (1) partition by list (a);
+create table fkpart2.fk_part_1_1 partition of fkpart2.fk_part_1 for values in (1);
+alter table fkpart2.fk_part_1 drop constraint p_fkey;	-- should fail
+alter table fkpart2.fk_part_1_1 drop constraint p_fkey;	-- should fail
+alter table fkpart2.fk_part detach partition fkpart2.fk_part_1;
+alter table fkpart2.fk_part_1 drop constraint p_fkey;
+
 \set VERBOSITY terse	\\ -- suppress cascade details
-drop schema fkpart0, fkpart1 cascade;
+drop schema fkpart0, fkpart1, fkpart2 cascade;
 \set VERBOSITY default
-- 
2.11.0

From 8eb5ce74e8656b517ad5a4e960b70de0ff3bedd5 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Thu, 24 Jan 2019 21:29:17 +0900
Subject: [PATCH 1/2] Fix ATAddForeignKeyConstraint to use correct value of
 connoinherit

While at it, add some Asserts to ConstraintSetParentConstraint to
assert the correct value of coninhcount.

Many tests fail, but the next patch should take care of them.
---
 src/backend/catalog/pg_constraint.c       | 11 +++++++++--
 src/backend/commands/tablecmds.c          |  5 ++++-
 src/test/regress/expected/foreign_key.out | 18 ++++++++++++++++--
 src/test/regress/sql/foreign_key.sql      | 15 ++++++++++++++-
 4 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index 446b54b9ff..d2b8489b6c 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -785,6 +785,12 @@ 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);
@@ -797,8 +803,9 @@ ConstraintSetParentConstraint(Oid childConstrId, Oid parentConstrId)
 	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,
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 738c178107..fea4d99735 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7251,6 +7251,9 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	bool		old_check_ok;
 	ObjectAddress address;
 	ListCell   *old_pfeqop_item = list_head(fkconstraint->old_conpfeqop);
+	/* Partitioned table's foreign key must always be inheritable. */
+	bool		connoinherit = (rel->rd_rel->relkind !=
+								RELKIND_PARTITIONED_TABLE);
 
 	/*
 	 * Grab ShareRowExclusiveLock on the pk table, so that someone doesn't
@@ -7616,7 +7619,7 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 									  NULL,
 									  true, /* islocal */
 									  0,	/* inhcount */
-									  true, /* isnoinherit */
+									  connoinherit, /* isnoinherit */
 									  false);	/* is_internal */
 	ObjectAddressSet(address, ConstraintRelationId, constrOid);
 
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index 76040a7601..b50bafef9e 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1945,7 +1945,21 @@ DETAIL:  Key (a)=(2) is not present in table "pkey".
 delete from fkpart1.pkey where a = 1;
 ERROR:  update or delete on table "pkey" violates foreign key constraint "fk_part_a_fkey" on table "fk_part_1"
 DETAIL:  Key (a)=(1) is still referenced from table "fk_part_1".
+-- verify that attaching and detaching partitions manipulates the inheritance
+-- properties of their FK constraints correctly
+create schema fkpart2;
+create table fkpart2.pkey (a int primary key);
+create table fkpart2.fk_part (a int, constraint fkey foreign key (a) references fkpart2.pkey) partition by list (a);
+create table fkpart2.fk_part_1 partition of fkpart2.fk_part for values in (1) partition by list (a);
+create table fkpart2.fk_part_1_1 (a int, constraint my_fkey foreign key (a) references fkpart2.pkey);
+alter table fkpart2.fk_part_1 attach partition fkpart2.fk_part_1_1 for values in (1);
+alter table fkpart2.fk_part_1 drop constraint fkey;	-- should fail
+ERROR:  cannot drop inherited constraint "fkey" of relation "fk_part_1"
+alter table fkpart2.fk_part_1_1 drop constraint my_fkey;	-- should fail
+ERROR:  cannot drop inherited constraint "my_fkey" of relation "fk_part_1_1"
+alter table fkpart2.fk_part detach partition fkpart2.fk_part_1;
+alter table fkpart2.fk_part_1 drop constraint fkey;
 \set VERBOSITY terse	\\ -- suppress cascade details
-drop schema fkpart0, fkpart1 cascade;
-NOTICE:  drop cascades to 5 other objects
+drop schema fkpart0, fkpart1, fkpart2 cascade;
+NOTICE:  drop cascades to 8 other objects
 \set VERBOSITY default
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 9ed1166c66..c02251eaa8 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -1392,6 +1392,19 @@ create table fkpart1.fk_part_1_2 partition of fkpart1.fk_part_1 for values in (2
 insert into fkpart1.fk_part_1 values (2);	-- should fail
 delete from fkpart1.pkey where a = 1;
 
+-- verify that attaching and detaching partitions manipulates the inheritance
+-- properties of their FK constraints correctly
+create schema fkpart2;
+create table fkpart2.pkey (a int primary key);
+create table fkpart2.fk_part (a int, constraint fkey foreign key (a) references fkpart2.pkey) partition by list (a);
+create table fkpart2.fk_part_1 partition of fkpart2.fk_part for values in (1) partition by list (a);
+create table fkpart2.fk_part_1_1 (a int, constraint my_fkey foreign key (a) references fkpart2.pkey);
+alter table fkpart2.fk_part_1 attach partition fkpart2.fk_part_1_1 for values in (1);
+alter table fkpart2.fk_part_1 drop constraint fkey;	-- should fail
+alter table fkpart2.fk_part_1_1 drop constraint my_fkey;	-- should fail
+alter table fkpart2.fk_part detach partition fkpart2.fk_part_1;
+alter table fkpart2.fk_part_1 drop constraint fkey;
+
 \set VERBOSITY terse	\\ -- suppress cascade details
-drop schema fkpart0, fkpart1 cascade;
+drop schema fkpart0, fkpart1, fkpart2 cascade;
 \set VERBOSITY default
-- 
2.11.0

From c2c9bd4c4976902525ebfcb0e32eb8249df549c9 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Thu, 24 Jan 2019 20:42:11 +0900
Subject: [PATCH 2/2] Don't recurse in ATExecDropConstraint for non-CHECK
 constraints

---
 src/backend/commands/tablecmds.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index fea4d99735..82e2cf1473 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -9134,6 +9134,7 @@ ATExecDropConstraint(Relation rel, const char *constrName,
 	HeapTuple	tuple;
 	bool		found = false;
 	bool		is_no_inherit_constraint = false;
+	char		contype;
 
 	/* At top level, permission check was done in ATPrepCmd, else do it */
 	if (recursing)
@@ -9203,6 +9204,7 @@ ATExecDropConstraint(Relation rel, const char *constrName,
 		performDeletion(&conobj, behavior, 0);
 
 		found = true;
+		contype = con->contype;
 	}
 
 	systable_endscan(scan);
@@ -9227,6 +9229,16 @@ ATExecDropConstraint(Relation rel, const char *constrName,
 	}
 
 	/*
+	 * Non-CHECK inherited constraints are dropped via dependency mechanism,
+	 * so we're done here for such constraints.
+	 */
+	if (contype != CONSTRAINT_CHECK)
+	{
+		table_close(conrel, RowExclusiveLock);
+		return;
+	}
+
+	/*
 	 * Propagate to children as appropriate.  Unlike most other ALTER
 	 * routines, we have to do this one level of recursion at a time; we can't
 	 * use find_all_inheritors to do it in one pass.
-- 
2.11.0

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 747acbd2b9..29860acaa2 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -417,7 +417,7 @@ static void CloneFkReferencing(Relation pg_constraint, Relation parentRel,
 static void ATExecDropConstraint(Relation rel, const char *constrName,
 					 DropBehavior behavior,
 					 bool recurse, bool recursing,
-					 bool missing_ok, LOCKMODE lockmode);
+					 bool missing_ok, LOCKMODE lockmode, ObjectAddresses *objsToDelete);
 static void ATPrepAlterColumnType(List **wqueue,
 					  AlteredTableInfo *tab, Relation rel,
 					  bool recurse, bool recursing,
@@ -4160,12 +4160,12 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
 		case AT_DropConstraint: /* DROP CONSTRAINT */
 			ATExecDropConstraint(rel, cmd->name, cmd->behavior,
 								 false, false,
-								 cmd->missing_ok, lockmode);
+								 cmd->missing_ok, lockmode, NULL);
 			break;
 		case AT_DropConstraintRecurse:	/* DROP CONSTRAINT with recursion */
 			ATExecDropConstraint(rel, cmd->name, cmd->behavior,
 								 true, false,
-								 cmd->missing_ok, lockmode);
+								 cmd->missing_ok, lockmode, NULL);
 			break;
 		case AT_AlterColumnType:	/* ALTER COLUMN TYPE */
 			address = ATExecAlterColumnType(tab, rel, cmd, lockmode);
@@ -9219,7 +9219,8 @@ static void
 ATExecDropConstraint(Relation rel, const char *constrName,
 					 DropBehavior behavior,
 					 bool recurse, bool recursing,
-					 bool missing_ok, LOCKMODE lockmode)
+					 bool missing_ok, LOCKMODE lockmode,
+					 ObjectAddresses *objsToDelete)
 {
 	List	   *children;
 	ListCell   *child;
@@ -9237,6 +9238,12 @@ ATExecDropConstraint(Relation rel, const char *constrName,
 
 	conrel = heap_open(ConstraintRelationId, RowExclusiveLock);
 
+	if (!recursing)
+	{
+		Assert(objsToDelete == NULL);
+		objsToDelete = new_object_addresses();
+	}
+
 	/*
 	 * Find and drop the target constraint
 	 */
@@ -9290,13 +9297,13 @@ ATExecDropConstraint(Relation rel, const char *constrName,
 		}
 
 		/*
-		 * Perform the actual constraint deletion
+		 * Register the constraint for deletion
 		 */
 		conobj.classId = ConstraintRelationId;
 		conobj.objectId = HeapTupleGetOid(tuple);
 		conobj.objectSubId = 0;
 
-		performDeletion(&conobj, behavior, 0);
+		add_exact_object_address(&conobj, objsToDelete);
 
 		found = true;
 	}
@@ -9402,7 +9409,7 @@ ATExecDropConstraint(Relation rel, const char *constrName,
 				/* Time to delete this child constraint, too */
 				ATExecDropConstraint(childrel, constrName, behavior,
 									 true, true,
-									 false, lockmode);
+									 false, lockmode, objsToDelete);
 			}
 			else
 			{
@@ -9435,6 +9442,9 @@ ATExecDropConstraint(Relation rel, const char *constrName,
 		heap_close(childrel, NoLock);
 	}
 
+	if (!recursing)
+		performMultipleDeletions(objsToDelete, behavior, 0);
+
 	heap_close(conrel, RowExclusiveLock);
 }