pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Foreign keys and partitioned tables
Date: Sun, 31 Dec 2017 16:49:08 -0300
Message-ID: <20171231194908.qbtfvrt77lgda4xf@alvherre.pgsql> (raw)
In-Reply-To: <20171231194359.cvojcour423ulha4@alvherre.pgsql>



-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From b7e85e873ba77509793180e9076295fae2fd88a7 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Tue, 21 Nov 2017 15:54:14 -0300
Subject: [PATCH 2/2] [WIP] Allow foreign key triggers on partitioned tables

---
 src/backend/catalog/pg_constraint.c        | 192 +++++++++++++++++++++++++++++
 src/backend/commands/tablecmds.c           | 105 +++++++++++++---
 src/backend/parser/parse_utilcmd.c         |  12 --
 src/backend/utils/adt/ri_triggers.c        |  50 +++-----
 src/include/catalog/pg_constraint_fn.h     |   2 +
 src/include/commands/tablecmds.h           |   4 +
 src/test/regress/expected/alter_table.out  | 115 ++++++++++++++++-
 src/test/regress/expected/create_table.out |  10 --
 src/test/regress/expected/foreign_key.out  |  67 ++++++++++
 src/test/regress/sql/alter_table.sql       |  57 ++++++++-
 src/test/regress/sql/create_table.sql      |   8 --
 src/test/regress/sql/foreign_key.sql       |  28 +++++
 12 files changed, 566 insertions(+), 84 deletions(-)

diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index 7dee6db0eb..9dc91fb67f 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -26,6 +26,7 @@
 #include "catalog/pg_operator.h"
 #include "catalog/pg_type.h"
 #include "commands/defrem.h"
+#include "commands/tablecmds.h"
 #include "utils/array.h"
 #include "utils/builtins.h"
 #include "utils/fmgroids.h"
@@ -375,6 +376,197 @@ CreateConstraintEntry(const char *constraintName,
 	return conOid;
 }
 
+/*
+ * For each foreign key constraint in relation parentId, create a cloned
+ * copy of it for relationId.
+ *
+ * relationId is a partition of parentId, so we can be certain that it has
+ * the same columns with the same datatypes.  They may be in different order,
+ * though.
+ */
+void
+CloneForeignKeyConstraints(Oid parentId, Oid relationId)
+{
+	Relation	pg_constraint;
+	Relation	rel;
+	ScanKeyData	key;
+	SysScanDesc	scan;
+	TupleDesc	tupdesc;
+	HeapTuple	tuple;
+
+	/* see ATAddForeignKeyConstraint about lock level */
+	rel = heap_open(relationId, AccessExclusiveLock);
+
+	pg_constraint = heap_open(ConstraintRelationId, RowShareLock);
+	tupdesc = RelationGetDescr(pg_constraint);
+
+	ScanKeyInit(&key,
+				Anum_pg_constraint_conrelid, BTEqualStrategyNumber,
+				F_OIDEQ, ObjectIdGetDatum(parentId));
+	scan = systable_beginscan(pg_constraint, ConstraintRelidIndexId, true,
+							  NULL, 1, &key);
+
+	while ((tuple = systable_getnext(scan)) != NULL)
+	{
+		Form_pg_constraint	constrForm = (Form_pg_constraint) GETSTRUCT(tuple);
+		AttrNumber	conkey[INDEX_MAX_KEYS];
+		AttrNumber	confkey[INDEX_MAX_KEYS];
+		Oid			conpfeqop[INDEX_MAX_KEYS];
+		Oid			conppeqop[INDEX_MAX_KEYS];
+		Oid			conffeqop[INDEX_MAX_KEYS];
+		Constraint *fkconstraint;
+		Oid			constrOid;
+		ObjectAddress parentAddr,
+					childAddr;
+		int			nelem;
+		ArrayType  *arr;
+		Datum		datum;
+		bool		isnull;
+
+		/* only foreign keys */
+		if (constrForm->contype != CONSTRAINT_FOREIGN)
+			continue;
+
+		ObjectAddressSet(parentAddr, ConstraintRelationId,
+						 HeapTupleGetOid(tuple));
+
+		datum = fastgetattr(tuple, Anum_pg_constraint_conkey,
+							tupdesc, &isnull);
+		if (isnull)
+			elog(ERROR, "null conkey");
+		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) != INT2OID)
+			elog(ERROR, "conkey is not a 1-D smallint array");
+		memcpy(conkey, ARR_DATA_PTR(arr), nelem * sizeof(AttrNumber));
+
+		datum = fastgetattr(tuple, Anum_pg_constraint_confkey,
+							tupdesc, &isnull);
+		if (isnull)
+			elog(ERROR, "null confkey");
+		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) != INT2OID)
+			elog(ERROR, "confkey is not a 1-D smallint array");
+		memcpy(confkey, ARR_DATA_PTR(arr), nelem * sizeof(AttrNumber));
+
+		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_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)
+			elog(ERROR, "null conppeqop");
+		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, "conppeqop is not a 1-D OID array");
+		memcpy(conppeqop, ARR_DATA_PTR(arr), nelem * sizeof(Oid));
+
+		datum = fastgetattr(tuple, Anum_pg_constraint_conffeqop,
+							tupdesc, &isnull);
+		if (isnull)
+			elog(ERROR, "null conffeqop");
+		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, "conffeqop is not a 1-D OID array");
+		memcpy(conffeqop, ARR_DATA_PTR(arr), nelem * sizeof(Oid));
+
+		constrOid =
+			CreateConstraintEntry(NameStr(constrForm->conname),
+								  constrForm->connamespace,
+								  CONSTRAINT_FOREIGN,
+								  constrForm->condeferrable,
+								  constrForm->condeferred,
+								  constrForm->convalidated,
+								  relationId,
+								  conkey,
+								  nelem,
+								  InvalidOid,	/* not a domain constraint */
+								  constrForm->conindid,	/* same index */
+								  constrForm->confrelid, /* same foreign rel */
+								  confkey,
+								  conpfeqop,
+								  conppeqop,
+								  conffeqop,
+								  nelem,
+								  constrForm->confupdtype,
+								  constrForm->confdeltype,
+								  constrForm->confmatchtype,
+								  NULL,
+								  NULL,
+								  NULL,
+								  NULL,
+								  false,
+								  1, false, true);
+
+		ObjectAddressSet(childAddr, ConstraintRelationId, constrOid);
+		recordDependencyOn(&childAddr, &parentAddr, DEPENDENCY_INTERNAL);
+
+		fkconstraint = makeNode(Constraint);
+		/* for now this is all we need */
+		fkconstraint->fk_upd_action = constrForm->confupdtype;
+		fkconstraint->fk_del_action = constrForm->confdeltype;
+		fkconstraint->deferrable = constrForm->condeferrable;
+		fkconstraint->initdeferred = constrForm->condeferred;
+
+		createForeignKeyTriggers(rel, constrForm->confrelid, fkconstraint,
+								 constrOid, constrForm->conindid);
+
+		/*
+		 * XXX Normal constraint creation can be invoked during ALTER and
+		 * so it needs ALTER TABLE's phase 3 checking.  Current caller is just
+		 * CREATE TABLE .. PARTITION OF so we don't need it, but maybe for
+		 * ALTER TABLE .. ATTACH PARTITION we'll need it.
+		 */
+	}
+	systable_endscan(scan);
+
+	heap_close(rel, NoLock);		/* keep lock till commit */
+	heap_close(pg_constraint, RowShareLock);
+}
 
 /*
  * Test whether given name is currently used as a constraint name
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index b0ddfb9110..d21f37216d 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -337,9 +337,6 @@ static void validateCheckConstraint(Relation rel, HeapTuple constrtup);
 static void validateForeignKeyConstraint(char *conname,
 							 Relation rel, Relation pkrel,
 							 Oid pkindOid, Oid constraintOid);
-static void createForeignKeyTriggers(Relation rel, Oid refRelOid,
-						 Constraint *fkconstraint,
-						 Oid constraintOid, Oid indexOid);
 static void ATController(AlterTableStmt *parsetree,
 			 Relation rel, List *cmds, bool recurse, LOCKMODE lockmode);
 static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
@@ -410,8 +407,10 @@ static ObjectAddress ATAddCheckConstraint(List **wqueue,
 					 Constraint *constr,
 					 bool recurse, bool recursing, bool is_readd,
 					 LOCKMODE lockmode);
-static ObjectAddress ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel,
-						  Constraint *fkconstraint, LOCKMODE lockmode);
+static ObjectAddress ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab,
+						  Relation rel,
+						  Constraint *fkconstraint, bool recurse, bool recursing,
+						  LOCKMODE lockmode);
 static void ATExecDropConstraint(Relation rel, const char *constrName,
 					 DropBehavior behavior,
 					 bool recurse, bool recursing,
@@ -503,6 +502,7 @@ static void refuseDupeIndexAttach(Relation parentIdx, Relation partIdx,
  * relkind: relkind to assign to the new relation
  * ownerId: if not InvalidOid, use this as the new relation's owner.
  * typaddress: if not null, it's set to the pg_type entry's address.
+ * queryString: for error reporting
  *
  * Note that permissions checks are done against current user regardless of
  * ownerId.  A nonzero ownerId is used when someone is creating a relation
@@ -958,6 +958,9 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 		}
 
 		list_free(idxlist);
+
+		CloneForeignKeyConstraints(parentId, relationId);
+
 		heap_close(parent, NoLock);
 	}
 
@@ -7043,7 +7046,8 @@ ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 										 RelationGetNamespace(rel),
 										 NIL);
 
-			address = ATAddForeignKeyConstraint(tab, rel, newConstraint,
+			address = ATAddForeignKeyConstraint(wqueue, tab, rel,
+												newConstraint, recurse, false,
 												lockmode);
 			break;
 
@@ -7198,8 +7202,9 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
  * We do permissions checks here, however.
  */
 static ObjectAddress
-ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel,
-						  Constraint *fkconstraint, LOCKMODE lockmode)
+ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
+						  Constraint *fkconstraint, bool recurse,
+						  bool recursing, LOCKMODE lockmode)
 {
 	Relation	pkrel;
 	int16		pkattnum[INDEX_MAX_KEYS];
@@ -7233,12 +7238,22 @@ ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel,
 	 * numbers)
 	 */
 	if (pkrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
-		ereport(ERROR,
-				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-				 errmsg("cannot reference partitioned table \"%s\"",
-						RelationGetRelationName(pkrel))));
+	{
+		if (!recurse)
+			ereport(ERROR,
+					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+					 errmsg("foreign key referencing partitioned table \"%s\" must not be ONLY",
+							RelationGetRelationName(pkrel))));
+		/* fix recursion in ATExecValidateConstraint to enable this case */
+		if (fkconstraint->skip_validation && !fkconstraint->initially_valid)
+			ereport(ERROR,
+					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+					 errmsg("cannot add NOT VALID foreign key to relation \"%s\"",
+							RelationGetRelationName(pkrel))));
+	}
 
-	if (pkrel->rd_rel->relkind != RELKIND_RELATION)
+	if (pkrel->rd_rel->relkind != RELKIND_RELATION &&
+		pkrel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
 		ereport(ERROR,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
 				 errmsg("referenced relation \"%s\" is not a table",
@@ -7598,6 +7613,45 @@ ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel,
 	}
 
 	/*
+	 * If this is a partitioned table, recurse to create the constraint on the
+	 * partitions also.
+	 */
+	if (recurse)
+	{
+		List	   *children;
+		ListCell   *child;
+
+		/* XXX why not find_all_inheritors? */
+		children = find_inheritance_children(RelationGetRelid(rel), lockmode);
+
+		foreach(child, children)
+		{
+			Oid			childrelid = lfirst_oid(child);
+			Relation	childrel;
+			AlteredTableInfo *childtab;
+			ObjectAddress childAddr;
+
+			/* find_inheritance_children already got lock */
+			childrel = heap_open(childrelid, NoLock);
+			CheckTableNotInUse(childrel, "ALTER TABLE");	/* XXX do we need this? */
+
+			/* Find or create work queue entry for this table */
+			childtab = ATGetQueueEntry(wqueue, childrel);
+
+			/* Recurse to child */
+			childAddr =
+				ATAddForeignKeyConstraint(wqueue, childtab, childrel,
+										  fkconstraint, recurse, true,
+										  lockmode);
+
+			/* make sure they go away together, or not at all */
+			recordDependencyOn(&childAddr, &address, DEPENDENCY_INTERNAL);
+
+			heap_close(childrel, NoLock);
+		}
+	}
+
+	/*
 	 * Close pk table, but keep lock until we've committed.
 	 */
 	heap_close(pkrel, NoLock);
@@ -7859,8 +7913,8 @@ ATExecValidateConstraint(Relation rel, char *constrName, bool recurse,
 			heap_close(refrel, NoLock);
 
 			/*
-			 * Foreign keys do not inherit, so we purposely ignore the
-			 * recursion bit here
+			 * We disallow creating invalid foreign keys to or from
+			 * partitioned tables, so ignoring the recursion bit is okay.
 			 */
 		}
 		else if (con->contype == CONSTRAINT_CHECK)
@@ -8511,7 +8565,7 @@ CreateFKCheckTrigger(Oid myRelOid, Oid refRelOid, Constraint *fkconstraint,
  * NB: if you change any trigger properties here, see also
  * ATExecAlterConstraint.
  */
-static void
+void
 createForeignKeyTriggers(Relation rel, Oid refRelOid, Constraint *fkconstraint,
 						 Oid constraintOid, Oid indexOid)
 {
@@ -8641,6 +8695,25 @@ createForeignKeyTriggers(Relation rel, Oid refRelOid, Constraint *fkconstraint,
 						 indexOid, true);
 	CreateFKCheckTrigger(myRelOid, refRelOid, fkconstraint, constraintOid,
 						 indexOid, false);
+
+	/*
+	 * If this is a partitioned table, recurse to create triggers for each
+	 * child.  We consider that one pg_constraint entry is enough; we only
+	 * need the triggers to appear per-partition.
+	 */
+	if (get_rel_relkind(refRelOid) == RELKIND_PARTITIONED_TABLE)
+	{
+		ListCell   *cell;
+		List	   *dchildren;
+
+		/* XXX maybe we need a stronger lock? */
+		dchildren = find_inheritance_children(refRelOid, RowShareLock);
+		foreach(cell, dchildren)
+		{
+			createForeignKeyTriggers(rel, lfirst_oid(cell), fkconstraint,
+									 constraintOid, indexOid);
+		}
+	}
 }
 
 /*
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 917b9bd636..19fd914b55 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -730,12 +730,6 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
 							 errmsg("foreign key constraints are not supported on foreign tables"),
 							 parser_errposition(cxt->pstate,
 												constraint->location)));
-				if (cxt->ispartitioned)
-					ereport(ERROR,
-							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-							 errmsg("foreign key constraints are not supported on partitioned tables"),
-							 parser_errposition(cxt->pstate,
-												constraint->location)));
 
 				/*
 				 * Fill in the current attribute's name and throw it into the
@@ -849,12 +843,6 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
 						 errmsg("foreign key constraints are not supported on foreign tables"),
 						 parser_errposition(cxt->pstate,
 											constraint->location)));
-			if (cxt->ispartitioned)
-				ereport(ERROR,
-						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-						 errmsg("foreign key constraints are not supported on partitioned tables"),
-						 parser_errposition(cxt->pstate,
-											constraint->location)));
 			cxt->fkconstraints = lappend(cxt->fkconstraints, constraint);
 			break;
 
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index b1ae9e5f96..f9c0e62417 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -401,7 +401,7 @@ RI_FKey_check(TriggerData *trigdata)
 
 		/* ----------
 		 * The query string built is
-		 *	SELECT 1 FROM ONLY <pktable> x WHERE pkatt1 = $1 [AND ...]
+		 *	SELECT 1 FROM <pktable> x WHERE pkatt1 = $1 [AND ...]
 		 *		   FOR KEY SHARE OF x
 		 * The type id's for the $ parameters are those of the
 		 * corresponding FK attributes.
@@ -409,7 +409,7 @@ RI_FKey_check(TriggerData *trigdata)
 		 */
 		initStringInfo(&querybuf);
 		quoteRelationName(pkrelname, pk_rel);
-		appendStringInfo(&querybuf, "SELECT 1 FROM ONLY %s x", pkrelname);
+		appendStringInfo(&querybuf, "SELECT 1 FROM %s x", pkrelname);
 		querysep = "WHERE";
 		for (i = 0; i < riinfo->nkeys; i++)
 		{
@@ -537,7 +537,7 @@ ri_Check_Pk_Match(Relation pk_rel, Relation fk_rel,
 
 		/* ----------
 		 * The query string built is
-		 *	SELECT 1 FROM ONLY <pktable> x WHERE pkatt1 = $1 [AND ...]
+		 *	SELECT 1 FROM <pktable> x WHERE pkatt1 = $1 [AND ...]
 		 *		   FOR KEY SHARE OF x
 		 * The type id's for the $ parameters are those of the
 		 * PK attributes themselves.
@@ -545,7 +545,7 @@ ri_Check_Pk_Match(Relation pk_rel, Relation fk_rel,
 		 */
 		initStringInfo(&querybuf);
 		quoteRelationName(pkrelname, pk_rel);
-		appendStringInfo(&querybuf, "SELECT 1 FROM ONLY %s x", pkrelname);
+		appendStringInfo(&querybuf, "SELECT 1 FROM %s x", pkrelname);
 		querysep = "WHERE";
 		for (i = 0; i < riinfo->nkeys; i++)
 		{
@@ -793,7 +793,7 @@ ri_restrict(TriggerData *trigdata, bool is_no_action)
 
 				/* ----------
 				 * The query string built is
-				 *	SELECT 1 FROM ONLY <fktable> x WHERE $1 = fkatt1 [AND ...]
+				 *	SELECT 1 FROM <fktable> x WHERE $1 = fkatt1 [AND ...]
 				 *		   FOR KEY SHARE OF x
 				 * The type id's for the $ parameters are those of the
 				 * corresponding PK attributes.
@@ -801,7 +801,7 @@ ri_restrict(TriggerData *trigdata, bool is_no_action)
 				 */
 				initStringInfo(&querybuf);
 				quoteRelationName(fkrelname, fk_rel);
-				appendStringInfo(&querybuf, "SELECT 1 FROM ONLY %s x",
+				appendStringInfo(&querybuf, "SELECT 1 FROM %s x",
 								 fkrelname);
 				querysep = "WHERE";
 				for (i = 0; i < riinfo->nkeys; i++)
@@ -951,14 +951,14 @@ RI_FKey_cascade_del(PG_FUNCTION_ARGS)
 
 				/* ----------
 				 * The query string built is
-				 *	DELETE FROM ONLY <fktable> WHERE $1 = fkatt1 [AND ...]
+				 *	DELETE FROM <fktable> WHERE $1 = fkatt1 [AND ...]
 				 * The type id's for the $ parameters are those of the
 				 * corresponding PK attributes.
 				 * ----------
 				 */
 				initStringInfo(&querybuf);
 				quoteRelationName(fkrelname, fk_rel);
-				appendStringInfo(&querybuf, "DELETE FROM ONLY %s", fkrelname);
+				appendStringInfo(&querybuf, "DELETE FROM %s", fkrelname);
 				querysep = "WHERE";
 				for (i = 0; i < riinfo->nkeys; i++)
 				{
@@ -1122,7 +1122,7 @@ RI_FKey_cascade_upd(PG_FUNCTION_ARGS)
 
 				/* ----------
 				 * The query string built is
-				 *	UPDATE ONLY <fktable> SET fkatt1 = $1 [, ...]
+				 *	UPDATE <fktable> SET fkatt1 = $1 [, ...]
 				 *			WHERE $n = fkatt1 [AND ...]
 				 * The type id's for the $ parameters are those of the
 				 * corresponding PK attributes.  Note that we are assuming
@@ -1133,7 +1133,7 @@ RI_FKey_cascade_upd(PG_FUNCTION_ARGS)
 				initStringInfo(&querybuf);
 				initStringInfo(&qualbuf);
 				quoteRelationName(fkrelname, fk_rel);
-				appendStringInfo(&querybuf, "UPDATE ONLY %s SET", fkrelname);
+				appendStringInfo(&querybuf, "UPDATE %s SET", fkrelname);
 				querysep = "";
 				qualsep = "WHERE";
 				for (i = 0, j = riinfo->nkeys; i < riinfo->nkeys; i++, j++)
@@ -1342,7 +1342,7 @@ ri_setnull(TriggerData *trigdata)
 
 				/* ----------
 				 * The query string built is
-				 *	UPDATE ONLY <fktable> SET fkatt1 = NULL [, ...]
+				 *	UPDATE <fktable> SET fkatt1 = NULL [, ...]
 				 *			WHERE $1 = fkatt1 [AND ...]
 				 * The type id's for the $ parameters are those of the
 				 * corresponding PK attributes.
@@ -1351,7 +1351,7 @@ ri_setnull(TriggerData *trigdata)
 				initStringInfo(&querybuf);
 				initStringInfo(&qualbuf);
 				quoteRelationName(fkrelname, fk_rel);
-				appendStringInfo(&querybuf, "UPDATE ONLY %s SET", fkrelname);
+				appendStringInfo(&querybuf, "UPDATE %s SET", fkrelname);
 				querysep = "";
 				qualsep = "WHERE";
 				for (i = 0; i < riinfo->nkeys; i++)
@@ -1559,7 +1559,7 @@ ri_setdefault(TriggerData *trigdata)
 
 				/* ----------
 				 * The query string built is
-				 *	UPDATE ONLY <fktable> SET fkatt1 = DEFAULT [, ...]
+				 *	UPDATE <fktable> SET fkatt1 = DEFAULT [, ...]
 				 *			WHERE $1 = fkatt1 [AND ...]
 				 * The type id's for the $ parameters are those of the
 				 * corresponding PK attributes.
@@ -1568,7 +1568,7 @@ ri_setdefault(TriggerData *trigdata)
 				initStringInfo(&querybuf);
 				initStringInfo(&qualbuf);
 				quoteRelationName(fkrelname, fk_rel);
-				appendStringInfo(&querybuf, "UPDATE ONLY %s SET", fkrelname);
+				appendStringInfo(&querybuf, "UPDATE %s SET", fkrelname);
 				querysep = "";
 				qualsep = "WHERE";
 				for (i = 0; i < riinfo->nkeys; i++)
@@ -1895,8 +1895,8 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
 
 	/*----------
 	 * The query string built is:
-	 *	SELECT fk.keycols FROM ONLY relname fk
-	 *	 LEFT OUTER JOIN ONLY pkrelname pk
+	 *	SELECT fk.keycols FROM relname fk
+	 *	 LEFT OUTER JOIN pkrelname pk
 	 *	 ON (pk.pkkeycol1=fk.keycol1 [AND ...])
 	 *	 WHERE pk.pkkeycol1 IS NULL AND
 	 * For MATCH SIMPLE:
@@ -1922,7 +1922,7 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel)
 	quoteRelationName(pkrelname, pk_rel);
 	quoteRelationName(fkrelname, fk_rel);
 	appendStringInfo(&querybuf,
-					 " FROM ONLY %s fk LEFT OUTER JOIN ONLY %s pk ON",
+					 " FROM %s fk LEFT OUTER JOIN %s pk ON",
 					 fkrelname, pkrelname);
 
 	strcpy(pkattname, "pk.");
@@ -2345,22 +2345,6 @@ ri_FetchConstraintInfo(Trigger *trigger, Relation trig_rel, bool rel_is_pk)
 	/* Find or create a hashtable entry for the constraint */
 	riinfo = ri_LoadConstraintInfo(constraintOid);
 
-	/* Do some easy cross-checks against the trigger call data */
-	if (rel_is_pk)
-	{
-		if (riinfo->fk_relid != trigger->tgconstrrelid ||
-			riinfo->pk_relid != RelationGetRelid(trig_rel))
-			elog(ERROR, "wrong pg_constraint entry for trigger \"%s\" on table \"%s\"",
-				 trigger->tgname, RelationGetRelationName(trig_rel));
-	}
-	else
-	{
-		if (riinfo->fk_relid != RelationGetRelid(trig_rel) ||
-			riinfo->pk_relid != trigger->tgconstrrelid)
-			elog(ERROR, "wrong pg_constraint entry for trigger \"%s\" on table \"%s\"",
-				 trigger->tgname, RelationGetRelationName(trig_rel));
-	}
-
 	return riinfo;
 }
 
diff --git a/src/include/catalog/pg_constraint_fn.h b/src/include/catalog/pg_constraint_fn.h
index 37b0b4ba82..438f5402fe 100644
--- a/src/include/catalog/pg_constraint_fn.h
+++ b/src/include/catalog/pg_constraint_fn.h
@@ -56,6 +56,8 @@ extern Oid CreateConstraintEntry(const char *constraintName,
 					  bool conNoInherit,
 					  bool is_internal);
 
+extern void CloneForeignKeyConstraints(Oid parentId, Oid relationId);
+
 extern void RemoveConstraintById(Oid conId);
 extern void RenameConstraintById(Oid conId, const char *newname);
 extern void SetValidatedConstraintById(Oid conId);
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index da3ff5dbee..c68515345b 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -74,6 +74,10 @@ extern void find_composite_type_dependencies(Oid typeOid,
 
 extern void check_of_type(HeapTuple typetuple);
 
+extern void createForeignKeyTriggers(Relation rel, Oid refRelOid,
+						 Constraint *fkconstraint, Oid constraintOid,
+						 Oid indexOid);
+
 extern void register_on_commit_action(Oid relid, OnCommitAction action);
 extern void remove_on_commit_action(Oid relid);
 
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 2caf930242..d4602d62ee 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -470,6 +470,117 @@ DROP TABLE tmp5;
 DROP TABLE tmp4;
 DROP TABLE tmp3;
 DROP TABLE tmp2;
+-- Ensure we can add foreign keys to and from partitioned tables
+SET search_path TO at_tst;
+CREATE SCHEMA at_tst;
+CREATE TABLE at_regular1 (col1 INT PRIMARY KEY);
+CREATE TABLE at_partitioned (col2 INT PRIMARY KEY,
+	reg1_col1 INT NOT NULL) PARTITION BY RANGE (col2);
+CREATE TABLE at_regular2 (col3 INT);
+ALTER TABLE at_regular2 ADD FOREIGN KEY (col3) REFERENCES at_partitioned;
+ALTER TABLE at_partitioned ADD FOREIGN KEY (reg1_col1) REFERENCES at_regular1;
+CREATE TABLE at_partitioned_0 PARTITION OF at_partitioned
+  FOR VALUES FROM (0) TO (10000);
+-- these fail:
+INSERT INTO at_regular2 VALUES (1000);
+ERROR:  insert or update on table "at_regular2" violates foreign key constraint "at_regular2_col3_fkey"
+DETAIL:  Key (col3)=(1000) is not present in table "at_partitioned".
+INSERT INTO at_partitioned VALUES (1000, 42);
+ERROR:  insert or update on table "at_partitioned_0" violates foreign key constraint "at_partitioned_reg1_col1_fkey"
+DETAIL:  Key (reg1_col1)=(42) is not present in table "at_regular1".
+-- these work:
+INSERT INTO at_regular1 VALUES (1000);
+INSERT INTO at_partitioned VALUES (42, 1000);
+INSERT INTO at_regular2 VALUES (42);
+CREATE TABLE at_partitioned_1 PARTITION OF at_partitioned
+  FOR VALUES FROM (10000) TO (20000);
+CREATE TABLE at_partitioned_2 (reg1_col1 INT, col2 INT);
+ALTER TABLE at_partitioned ATTACH PARTITION at_partitioned_2
+  FOR VALUES FROM (20000) TO (30000);
+ERROR:  column "col2" in child table must be marked NOT NULL
+ALTER TABLE at_partitioned_2
+	ALTER col2 SET NOT NULL,
+	ALTER reg1_col1 SET NOT NULL;
+ALTER TABLE at_partitioned ATTACH PARTITION at_partitioned_2
+  FOR VALUES FROM (20000) TO (30000);
+\d at_regular2
+            Table "at_tst.at_regular2"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ col3   | integer |           |          | 
+Foreign-key constraints:
+    "at_regular2_col3_fkey" FOREIGN KEY (col3) REFERENCES at_partitioned(col2)
+
+\d at_partitioned
+            Table "at_tst.at_partitioned"
+  Column   |  Type   | Collation | Nullable | Default 
+-----------+---------+-----------+----------+---------
+ col2      | integer |           | not null | 
+ reg1_col1 | integer |           | not null | 
+Partition key: RANGE (col2)
+Indexes:
+    "at_partitioned_pkey" PRIMARY KEY, btree (col2)
+Foreign-key constraints:
+    "at_partitioned_reg1_col1_fkey" FOREIGN KEY (reg1_col1) REFERENCES at_regular1(col1)
+Referenced by:
+    TABLE "at_regular2" CONSTRAINT "at_regular2_col3_fkey" FOREIGN KEY (col3) REFERENCES at_partitioned(col2)
+Number of partitions: 3 (Use \d+ to list them.)
+
+\d at_partitioned_0
+           Table "at_tst.at_partitioned_0"
+  Column   |  Type   | Collation | Nullable | Default 
+-----------+---------+-----------+----------+---------
+ col2      | integer |           | not null | 
+ reg1_col1 | integer |           | not null | 
+Partition of: at_partitioned FOR VALUES FROM (0) TO (10000)
+Indexes:
+    "at_partitioned_0_pkey" PRIMARY KEY, btree (col2)
+Foreign-key constraints:
+    "at_partitioned_reg1_col1_fkey" FOREIGN KEY (reg1_col1) REFERENCES at_regular1(col1)
+
+\d at_partitioned_1
+           Table "at_tst.at_partitioned_1"
+  Column   |  Type   | Collation | Nullable | Default 
+-----------+---------+-----------+----------+---------
+ col2      | integer |           | not null | 
+ reg1_col1 | integer |           | not null | 
+Partition of: at_partitioned FOR VALUES FROM (10000) TO (20000)
+Indexes:
+    "at_partitioned_1_pkey" PRIMARY KEY, btree (col2)
+Foreign-key constraints:
+    "at_partitioned_reg1_col1_fkey" FOREIGN KEY (reg1_col1) REFERENCES at_regular1(col1)
+
+\d at_partitioned_2
+           Table "at_tst.at_partitioned_2"
+  Column   |  Type   | Collation | Nullable | Default 
+-----------+---------+-----------+----------+---------
+ reg1_col1 | integer |           | not null | 
+ col2      | integer |           | not null | 
+Partition of: at_partitioned FOR VALUES FROM (20000) TO (30000)
+Indexes:
+    "at_partitioned_2_pkey" PRIMARY KEY, btree (col2)
+
+INSERT INTO at_partitioned VALUES (5000, 42);
+ERROR:  insert or update on table "at_partitioned_0" violates foreign key constraint "at_partitioned_reg1_col1_fkey"
+DETAIL:  Key (reg1_col1)=(42) is not present in table "at_regular1".
+INSERT INTO at_regular1 VALUES (42), (1042), (2042);
+INSERT INTO at_partitioned VALUES (5000, 42), (15000, 1042), (25000, 2042);
+INSERT INTO at_regular2 VALUES (5000), (15000), (25000);
+INSERT INTO at_regular2 VALUES (35000);
+ERROR:  insert or update on table "at_regular2" violates foreign key constraint "at_regular2_col3_fkey"
+DETAIL:  Key (col3)=(35000) is not present in table "at_partitioned".
+-- ok
+ALTER TABLE at_regular2 DROP CONSTRAINT at_regular2_col3_fkey;
+-- disallowed: must drop it from parent instead
+ALTER TABLE at_partitioned_0 DROP CONSTRAINT at_partitioned_reg1_col1_fkey;
+ERROR:  cannot drop inherited constraint "at_partitioned_reg1_col1_fkey" of relation "at_partitioned_0"
+-- ok
+ALTER TABLE at_partitioned DROP CONSTRAINT at_partitioned_reg1_col1_fkey;
+\set VERBOSITY terse
+DROP SCHEMA at_tst CASCADE;
+NOTICE:  drop cascades to 3 other objects
+\set VERBOSITY default
+RESET search_path;
 -- NOT VALID with plan invalidation -- ensure we don't use a constraint for
 -- exclusion until validated
 set constraint_exclusion TO 'partition';
@@ -3290,10 +3401,6 @@ CREATE TABLE partitioned (
 	a int,
 	b int
 ) PARTITION BY RANGE (a, (a+b+1));
-ALTER TABLE partitioned ADD FOREIGN KEY (a) REFERENCES blah;
-ERROR:  foreign key constraints are not supported on partitioned tables
-LINE 1: ALTER TABLE partitioned ADD FOREIGN KEY (a) REFERENCES blah;
-                                    ^
 ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&);
 ERROR:  exclusion constraints are not supported on partitioned tables
 LINE 1: ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&);
diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out
index 866cc99b9f..7c3703b73d 100644
--- a/src/test/regress/expected/create_table.out
+++ b/src/test/regress/expected/create_table.out
@@ -276,16 +276,6 @@ CREATE TABLE partitioned (
 ) PARTITION BY LIST (a1, a2);	-- fail
 ERROR:  cannot use "list" partition strategy with more than one column
 -- unsupported constraint type for partitioned tables
-CREATE TABLE pkrel (
-	a int PRIMARY KEY
-);
-CREATE TABLE partitioned (
-	a int REFERENCES pkrel(a)
-) PARTITION BY RANGE (a);
-ERROR:  foreign key constraints are not supported on partitioned tables
-LINE 2:  a int REFERENCES pkrel(a)
-               ^
-DROP TABLE pkrel;
 CREATE TABLE partitioned (
 	a int,
 	EXCLUDE USING gist (a WITH &&)
diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out
index fef072eddf..8c71c21973 100644
--- a/src/test/regress/expected/foreign_key.out
+++ b/src/test/regress/expected/foreign_key.out
@@ -1415,3 +1415,70 @@ alter table fktable2 drop constraint fktable2_f1_fkey;
 ERROR:  cannot ALTER TABLE "pktable2" because it has pending trigger events
 commit;
 drop table pktable2, fktable2;
+--
+-- Foreign keys and partitioned tables
+--
+-- Test that it's possible to have a FK from a partitioned table to a regular
+-- one
+CREATE TABLE pkregular (f1 int primary key);
+CREATE TABLE fkpartit (f1 int references pkregular) PARTITION BY RANGE (f1);
+CREATE TABLE fkpart1  PARTITION OF fkpartit FOR VALUES FROM (0) TO (1000);
+INSERT INTO fkpartit VALUES (500);
+ERROR:  insert or update on table "fkpart1" violates foreign key constraint "fkpartit_f1_fkey"
+DETAIL:  Key (f1)=(500) is not present in table "pkregular".
+INSERT INTO fkpart1 VALUES (500);
+ERROR:  insert or update on table "fkpart1" violates foreign key constraint "fkpartit_f1_fkey"
+DETAIL:  Key (f1)=(500) is not present in table "pkregular".
+INSERT INTO pkregular VALUES (500);
+INSERT INTO fkpartit VALUES (500);
+INSERT INTO fkpart1 VALUES (500);
+DELETE FROM pkregular;
+ERROR:  update or delete on table "pkregular" violates foreign key constraint "fkpartit_f1_fkey" on table "fkpartit"
+DETAIL:  Key (f1)=(500) is still referenced from table "fkpartit".
+UPDATE pkregular SET f1 = 501;
+ERROR:  update or delete on table "pkregular" violates foreign key constraint "fkpartit_f1_fkey" on table "fkpartit"
+DETAIL:  Key (f1)=(500) is still referenced from table "fkpartit".
+ALTER TABLE fkpart1 DROP CONSTRAINT fkpartit_f1_fkey;	-- nope
+ERROR:  cannot drop inherited constraint "fkpartit_f1_fkey" of relation "fkpart1"
+ALTER TABLE fkpartit DROP CONSTRAINT fkpartit_f1_fkey;
+\d fkpartit
+              Table "public.fkpartit"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ f1     | integer |           |          | 
+Partition key: RANGE (f1)
+Number of partitions: 1 (Use \d+ to list them.)
+
+\d fkpart1
+              Table "public.fkpart1"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ f1     | integer |           |          | 
+Partition of: fkpartit FOR VALUES FROM (0) TO (1000)
+
+ALTER TABLE fkpartit ADD CONSTRAINT fkpartit_f1_fkey FOREIGN KEY (f1) REFERENCES pkregular ON DELETE CASCADE;
+\d fkpartit
+              Table "public.fkpartit"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ f1     | integer |           |          | 
+Partition key: RANGE (f1)
+Foreign-key constraints:
+    "fkpartit_f1_fkey" FOREIGN KEY (f1) REFERENCES pkregular(f1) ON DELETE CASCADE
+Number of partitions: 1 (Use \d+ to list them.)
+
+\d fkpart1
+              Table "public.fkpart1"
+ Column |  Type   | Collation | Nullable | Default 
+--------+---------+-----------+----------+---------
+ f1     | integer |           |          | 
+Partition of: fkpartit FOR VALUES FROM (0) TO (1000)
+Foreign-key constraints:
+    "fkpartit_f1_fkey" FOREIGN KEY (f1) REFERENCES pkregular(f1) ON DELETE CASCADE
+
+DELETE FROM pkregular;
+SELECT * FROM fkpartit;
+ f1 
+----
+(0 rows)
+
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index ed0bb7845b..40b3d0256b 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -375,6 +375,62 @@ DROP TABLE tmp3;
 
 DROP TABLE tmp2;
 
+-- Ensure we can add foreign keys to and from partitioned tables
+SET search_path TO at_tst;
+CREATE SCHEMA at_tst;
+CREATE TABLE at_regular1 (col1 INT PRIMARY KEY);
+CREATE TABLE at_partitioned (col2 INT PRIMARY KEY,
+	reg1_col1 INT NOT NULL) PARTITION BY RANGE (col2);
+CREATE TABLE at_regular2 (col3 INT);
+ALTER TABLE at_regular2 ADD FOREIGN KEY (col3) REFERENCES at_partitioned;
+ALTER TABLE at_partitioned ADD FOREIGN KEY (reg1_col1) REFERENCES at_regular1;
+CREATE TABLE at_partitioned_0 PARTITION OF at_partitioned
+  FOR VALUES FROM (0) TO (10000);
+-- these fail:
+INSERT INTO at_regular2 VALUES (1000);
+INSERT INTO at_partitioned VALUES (1000, 42);
+
+-- these work:
+INSERT INTO at_regular1 VALUES (1000);
+INSERT INTO at_partitioned VALUES (42, 1000);
+INSERT INTO at_regular2 VALUES (42);
+
+CREATE TABLE at_partitioned_1 PARTITION OF at_partitioned
+  FOR VALUES FROM (10000) TO (20000);
+CREATE TABLE at_partitioned_2 (reg1_col1 INT, col2 INT);
+ALTER TABLE at_partitioned ATTACH PARTITION at_partitioned_2
+  FOR VALUES FROM (20000) TO (30000);
+ALTER TABLE at_partitioned_2
+	ALTER col2 SET NOT NULL,
+	ALTER reg1_col1 SET NOT NULL;
+ALTER TABLE at_partitioned ATTACH PARTITION at_partitioned_2
+  FOR VALUES FROM (20000) TO (30000);
+
+\d at_regular2
+\d at_partitioned
+\d at_partitioned_0
+\d at_partitioned_1
+\d at_partitioned_2
+
+INSERT INTO at_partitioned VALUES (5000, 42);
+INSERT INTO at_regular1 VALUES (42), (1042), (2042);
+INSERT INTO at_partitioned VALUES (5000, 42), (15000, 1042), (25000, 2042);
+INSERT INTO at_regular2 VALUES (5000), (15000), (25000);
+INSERT INTO at_regular2 VALUES (35000);
+
+-- ok
+ALTER TABLE at_regular2 DROP CONSTRAINT at_regular2_col3_fkey;
+
+-- disallowed: must drop it from parent instead
+ALTER TABLE at_partitioned_0 DROP CONSTRAINT at_partitioned_reg1_col1_fkey;
+-- ok
+ALTER TABLE at_partitioned DROP CONSTRAINT at_partitioned_reg1_col1_fkey;
+
+\set VERBOSITY terse
+DROP SCHEMA at_tst CASCADE;
+\set VERBOSITY default
+RESET search_path;
+
 -- NOT VALID with plan invalidation -- ensure we don't use a constraint for
 -- exclusion until validated
 set constraint_exclusion TO 'partition';
@@ -2016,7 +2072,6 @@ CREATE TABLE partitioned (
 	a int,
 	b int
 ) PARTITION BY RANGE (a, (a+b+1));
-ALTER TABLE partitioned ADD FOREIGN KEY (a) REFERENCES blah;
 ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&);
 
 -- cannot drop column that is part of the partition key
diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql
index fefccf21a2..09a634d79d 100644
--- a/src/test/regress/sql/create_table.sql
+++ b/src/test/regress/sql/create_table.sql
@@ -294,14 +294,6 @@ CREATE TABLE partitioned (
 ) PARTITION BY LIST (a1, a2);	-- fail
 
 -- unsupported constraint type for partitioned tables
-CREATE TABLE pkrel (
-	a int PRIMARY KEY
-);
-CREATE TABLE partitioned (
-	a int REFERENCES pkrel(a)
-) PARTITION BY RANGE (a);
-DROP TABLE pkrel;
-
 CREATE TABLE partitioned (
 	a int,
 	EXCLUDE USING gist (a WITH &&)
diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql
index 5f19dad03c..1a2c93fcce 100644
--- a/src/test/regress/sql/foreign_key.sql
+++ b/src/test/regress/sql/foreign_key.sql
@@ -1055,3 +1055,31 @@ alter table fktable2 drop constraint fktable2_f1_fkey;
 commit;
 
 drop table pktable2, fktable2;
+
+
+--
+-- Foreign keys and partitioned tables
+--
+
+-- Test that it's possible to have a FK from a partitioned table to a regular
+-- one
+CREATE TABLE pkregular (f1 int primary key);
+CREATE TABLE fkpartit (f1 int references pkregular) PARTITION BY RANGE (f1);
+CREATE TABLE fkpart1  PARTITION OF fkpartit FOR VALUES FROM (0) TO (1000);
+INSERT INTO fkpartit VALUES (500);
+INSERT INTO fkpart1 VALUES (500);
+INSERT INTO pkregular VALUES (500);
+INSERT INTO fkpartit VALUES (500);
+INSERT INTO fkpart1 VALUES (500);
+DELETE FROM pkregular;
+UPDATE pkregular SET f1 = 501;
+
+ALTER TABLE fkpart1 DROP CONSTRAINT fkpartit_f1_fkey;	-- nope
+ALTER TABLE fkpartit DROP CONSTRAINT fkpartit_f1_fkey;
+\d fkpartit
+\d fkpart1
+ALTER TABLE fkpartit ADD CONSTRAINT fkpartit_f1_fkey FOREIGN KEY (f1) REFERENCES pkregular ON DELETE CASCADE;
+\d fkpartit
+\d fkpart1
+DELETE FROM pkregular;
+SELECT * FROM fkpartit;
-- 
2.11.0

Attachments:

  0001-Local-partitioned-indexes.patch.gz (application/x-gunzip, 42.6K) download

0002-WIP-Allow-foreign-key-triggers-on-partitioned-tables.patch (text/plain, 38.1K) download From b7e85e873ba77509793180e9076295fae2fd88a7 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera <alvherre@alvh.no-ip.org> Date: Tue, 21 Nov 2017 15:54:14 -0300 Subject: [PATCH 2/2] [WIP] Allow foreign key triggers on partitioned tables --- src/backend/catalog/pg_constraint.c | 192 +++++++++++++++++++++++++++++ src/backend/commands/tablecmds.c | 105 +++++++++++++--- src/backend/parser/parse_utilcmd.c | 12 -- src/backend/utils/adt/ri_triggers.c | 50 +++----- src/include/catalog/pg_constraint_fn.h | 2 + src/include/commands/tablecmds.h | 4 + src/test/regress/expected/alter_table.out | 115 ++++++++++++++++- src/test/regress/expected/create_table.out | 10 -- src/test/regress/expected/foreign_key.out | 67 ++++++++++ src/test/regress/sql/alter_table.sql | 57 ++++++++- src/test/regress/sql/create_table.sql | 8 -- src/test/regress/sql/foreign_key.sql | 28 +++++ 12 files changed, 566 insertions(+), 84 deletions(-) diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c index 7dee6db0eb..9dc91fb67f 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -26,6 +26,7 @@ #include "catalog/pg_operator.h" #include "catalog/pg_type.h" #include "commands/defrem.h" +#include "commands/tablecmds.h" #include "utils/array.h" #include "utils/builtins.h" #include "utils/fmgroids.h" @@ -375,6 +376,197 @@ CreateConstraintEntry(const char *constraintName, return conOid; } +/* + * For each foreign key constraint in relation parentId, create a cloned + * copy of it for relationId. + * + * relationId is a partition of parentId, so we can be certain that it has + * the same columns with the same datatypes. They may be in different order, + * though. + */ +void +CloneForeignKeyConstraints(Oid parentId, Oid relationId) +{ + Relation pg_constraint; + Relation rel; + ScanKeyData key; + SysScanDesc scan; + TupleDesc tupdesc; + HeapTuple tuple; + + /* see ATAddForeignKeyConstraint about lock level */ + rel = heap_open(relationId, AccessExclusiveLock); + + pg_constraint = heap_open(ConstraintRelationId, RowShareLock); + tupdesc = RelationGetDescr(pg_constraint); + + ScanKeyInit(&key, + Anum_pg_constraint_conrelid, BTEqualStrategyNumber, + F_OIDEQ, ObjectIdGetDatum(parentId)); + scan = systable_beginscan(pg_constraint, ConstraintRelidIndexId, true, + NULL, 1, &key); + + while ((tuple = systable_getnext(scan)) != NULL) + { + Form_pg_constraint constrForm = (Form_pg_constraint) GETSTRUCT(tuple); + AttrNumber conkey[INDEX_MAX_KEYS]; + AttrNumber confkey[INDEX_MAX_KEYS]; + Oid conpfeqop[INDEX_MAX_KEYS]; + Oid conppeqop[INDEX_MAX_KEYS]; + Oid conffeqop[INDEX_MAX_KEYS]; + Constraint *fkconstraint; + Oid constrOid; + ObjectAddress parentAddr, + childAddr; + int nelem; + ArrayType *arr; + Datum datum; + bool isnull; + + /* only foreign keys */ + if (constrForm->contype != CONSTRAINT_FOREIGN) + continue; + + ObjectAddressSet(parentAddr, ConstraintRelationId, + HeapTupleGetOid(tuple)); + + datum = fastgetattr(tuple, Anum_pg_constraint_conkey, + tupdesc, &isnull); + if (isnull) + elog(ERROR, "null conkey"); + 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) != INT2OID) + elog(ERROR, "conkey is not a 1-D smallint array"); + memcpy(conkey, ARR_DATA_PTR(arr), nelem * sizeof(AttrNumber)); + + datum = fastgetattr(tuple, Anum_pg_constraint_confkey, + tupdesc, &isnull); + if (isnull) + elog(ERROR, "null confkey"); + 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) != INT2OID) + elog(ERROR, "confkey is not a 1-D smallint array"); + memcpy(confkey, ARR_DATA_PTR(arr), nelem * sizeof(AttrNumber)); + + 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_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) + elog(ERROR, "null conppeqop"); + 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, "conppeqop is not a 1-D OID array"); + memcpy(conppeqop, ARR_DATA_PTR(arr), nelem * sizeof(Oid)); + + datum = fastgetattr(tuple, Anum_pg_constraint_conffeqop, + tupdesc, &isnull); + if (isnull) + elog(ERROR, "null conffeqop"); + 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, "conffeqop is not a 1-D OID array"); + memcpy(conffeqop, ARR_DATA_PTR(arr), nelem * sizeof(Oid)); + + constrOid = + CreateConstraintEntry(NameStr(constrForm->conname), + constrForm->connamespace, + CONSTRAINT_FOREIGN, + constrForm->condeferrable, + constrForm->condeferred, + constrForm->convalidated, + relationId, + conkey, + nelem, + InvalidOid, /* not a domain constraint */ + constrForm->conindid, /* same index */ + constrForm->confrelid, /* same foreign rel */ + confkey, + conpfeqop, + conppeqop, + conffeqop, + nelem, + constrForm->confupdtype, + constrForm->confdeltype, + constrForm->confmatchtype, + NULL, + NULL, + NULL, + NULL, + false, + 1, false, true); + + ObjectAddressSet(childAddr, ConstraintRelationId, constrOid); + recordDependencyOn(&childAddr, &parentAddr, DEPENDENCY_INTERNAL); + + fkconstraint = makeNode(Constraint); + /* for now this is all we need */ + fkconstraint->fk_upd_action = constrForm->confupdtype; + fkconstraint->fk_del_action = constrForm->confdeltype; + fkconstraint->deferrable = constrForm->condeferrable; + fkconstraint->initdeferred = constrForm->condeferred; + + createForeignKeyTriggers(rel, constrForm->confrelid, fkconstraint, + constrOid, constrForm->conindid); + + /* + * XXX Normal constraint creation can be invoked during ALTER and + * so it needs ALTER TABLE's phase 3 checking. Current caller is just + * CREATE TABLE .. PARTITION OF so we don't need it, but maybe for + * ALTER TABLE .. ATTACH PARTITION we'll need it. + */ + } + systable_endscan(scan); + + heap_close(rel, NoLock); /* keep lock till commit */ + heap_close(pg_constraint, RowShareLock); +} /* * Test whether given name is currently used as a constraint name diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index b0ddfb9110..d21f37216d 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -337,9 +337,6 @@ static void validateCheckConstraint(Relation rel, HeapTuple constrtup); static void validateForeignKeyConstraint(char *conname, Relation rel, Relation pkrel, Oid pkindOid, Oid constraintOid); -static void createForeignKeyTriggers(Relation rel, Oid refRelOid, - Constraint *fkconstraint, - Oid constraintOid, Oid indexOid); static void ATController(AlterTableStmt *parsetree, Relation rel, List *cmds, bool recurse, LOCKMODE lockmode); static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, @@ -410,8 +407,10 @@ static ObjectAddress ATAddCheckConstraint(List **wqueue, Constraint *constr, bool recurse, bool recursing, bool is_readd, LOCKMODE lockmode); -static ObjectAddress ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel, - Constraint *fkconstraint, LOCKMODE lockmode); +static ObjectAddress ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, + Relation rel, + Constraint *fkconstraint, bool recurse, bool recursing, + LOCKMODE lockmode); static void ATExecDropConstraint(Relation rel, const char *constrName, DropBehavior behavior, bool recurse, bool recursing, @@ -503,6 +502,7 @@ static void refuseDupeIndexAttach(Relation parentIdx, Relation partIdx, * relkind: relkind to assign to the new relation * ownerId: if not InvalidOid, use this as the new relation's owner. * typaddress: if not null, it's set to the pg_type entry's address. + * queryString: for error reporting * * Note that permissions checks are done against current user regardless of * ownerId. A nonzero ownerId is used when someone is creating a relation @@ -958,6 +958,9 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, } list_free(idxlist); + + CloneForeignKeyConstraints(parentId, relationId); + heap_close(parent, NoLock); } @@ -7043,7 +7046,8 @@ ATExecAddConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, RelationGetNamespace(rel), NIL); - address = ATAddForeignKeyConstraint(tab, rel, newConstraint, + address = ATAddForeignKeyConstraint(wqueue, tab, rel, + newConstraint, recurse, false, lockmode); break; @@ -7198,8 +7202,9 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, * We do permissions checks here, however. */ static ObjectAddress -ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel, - Constraint *fkconstraint, LOCKMODE lockmode) +ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, + Constraint *fkconstraint, bool recurse, + bool recursing, LOCKMODE lockmode) { Relation pkrel; int16 pkattnum[INDEX_MAX_KEYS]; @@ -7233,12 +7238,22 @@ ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel, * numbers) */ if (pkrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) - ereport(ERROR, - (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("cannot reference partitioned table \"%s\"", - RelationGetRelationName(pkrel)))); + { + if (!recurse) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("foreign key referencing partitioned table \"%s\" must not be ONLY", + RelationGetRelationName(pkrel)))); + /* fix recursion in ATExecValidateConstraint to enable this case */ + if (fkconstraint->skip_validation && !fkconstraint->initially_valid) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("cannot add NOT VALID foreign key to relation \"%s\"", + RelationGetRelationName(pkrel)))); + } - if (pkrel->rd_rel->relkind != RELKIND_RELATION) + if (pkrel->rd_rel->relkind != RELKIND_RELATION && + pkrel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("referenced relation \"%s\" is not a table", @@ -7598,6 +7613,45 @@ ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel, } /* + * If this is a partitioned table, recurse to create the constraint on the + * partitions also. + */ + if (recurse) + { + List *children; + ListCell *child; + + /* XXX why not find_all_inheritors? */ + children = find_inheritance_children(RelationGetRelid(rel), lockmode); + + foreach(child, children) + { + Oid childrelid = lfirst_oid(child); + Relation childrel; + AlteredTableInfo *childtab; + ObjectAddress childAddr; + + /* find_inheritance_children already got lock */ + childrel = heap_open(childrelid, NoLock); + CheckTableNotInUse(childrel, "ALTER TABLE"); /* XXX do we need this? */ + + /* Find or create work queue entry for this table */ + childtab = ATGetQueueEntry(wqueue, childrel); + + /* Recurse to child */ + childAddr = + ATAddForeignKeyConstraint(wqueue, childtab, childrel, + fkconstraint, recurse, true, + lockmode); + + /* make sure they go away together, or not at all */ + recordDependencyOn(&childAddr, &address, DEPENDENCY_INTERNAL); + + heap_close(childrel, NoLock); + } + } + + /* * Close pk table, but keep lock until we've committed. */ heap_close(pkrel, NoLock); @@ -7859,8 +7913,8 @@ ATExecValidateConstraint(Relation rel, char *constrName, bool recurse, heap_close(refrel, NoLock); /* - * Foreign keys do not inherit, so we purposely ignore the - * recursion bit here + * We disallow creating invalid foreign keys to or from + * partitioned tables, so ignoring the recursion bit is okay. */ } else if (con->contype == CONSTRAINT_CHECK) @@ -8511,7 +8565,7 @@ CreateFKCheckTrigger(Oid myRelOid, Oid refRelOid, Constraint *fkconstraint, * NB: if you change any trigger properties here, see also * ATExecAlterConstraint. */ -static void +void createForeignKeyTriggers(Relation rel, Oid refRelOid, Constraint *fkconstraint, Oid constraintOid, Oid indexOid) { @@ -8641,6 +8695,25 @@ createForeignKeyTriggers(Relation rel, Oid refRelOid, Constraint *fkconstraint, indexOid, true); CreateFKCheckTrigger(myRelOid, refRelOid, fkconstraint, constraintOid, indexOid, false); + + /* + * If this is a partitioned table, recurse to create triggers for each + * child. We consider that one pg_constraint entry is enough; we only + * need the triggers to appear per-partition. + */ + if (get_rel_relkind(refRelOid) == RELKIND_PARTITIONED_TABLE) + { + ListCell *cell; + List *dchildren; + + /* XXX maybe we need a stronger lock? */ + dchildren = find_inheritance_children(refRelOid, RowShareLock); + foreach(cell, dchildren) + { + createForeignKeyTriggers(rel, lfirst_oid(cell), fkconstraint, + constraintOid, indexOid); + } + } } /* diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 917b9bd636..19fd914b55 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -730,12 +730,6 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column) errmsg("foreign key constraints are not supported on foreign tables"), parser_errposition(cxt->pstate, constraint->location))); - if (cxt->ispartitioned) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("foreign key constraints are not supported on partitioned tables"), - parser_errposition(cxt->pstate, - constraint->location))); /* * Fill in the current attribute's name and throw it into the @@ -849,12 +843,6 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint) errmsg("foreign key constraints are not supported on foreign tables"), parser_errposition(cxt->pstate, constraint->location))); - if (cxt->ispartitioned) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("foreign key constraints are not supported on partitioned tables"), - parser_errposition(cxt->pstate, - constraint->location))); cxt->fkconstraints = lappend(cxt->fkconstraints, constraint); break; diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index b1ae9e5f96..f9c0e62417 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -401,7 +401,7 @@ RI_FKey_check(TriggerData *trigdata) /* ---------- * The query string built is - * SELECT 1 FROM ONLY <pktable> x WHERE pkatt1 = $1 [AND ...] + * SELECT 1 FROM <pktable> x WHERE pkatt1 = $1 [AND ...] * FOR KEY SHARE OF x * The type id's for the $ parameters are those of the * corresponding FK attributes. @@ -409,7 +409,7 @@ RI_FKey_check(TriggerData *trigdata) */ initStringInfo(&querybuf); quoteRelationName(pkrelname, pk_rel); - appendStringInfo(&querybuf, "SELECT 1 FROM ONLY %s x", pkrelname); + appendStringInfo(&querybuf, "SELECT 1 FROM %s x", pkrelname); querysep = "WHERE"; for (i = 0; i < riinfo->nkeys; i++) { @@ -537,7 +537,7 @@ ri_Check_Pk_Match(Relation pk_rel, Relation fk_rel, /* ---------- * The query string built is - * SELECT 1 FROM ONLY <pktable> x WHERE pkatt1 = $1 [AND ...] + * SELECT 1 FROM <pktable> x WHERE pkatt1 = $1 [AND ...] * FOR KEY SHARE OF x * The type id's for the $ parameters are those of the * PK attributes themselves. @@ -545,7 +545,7 @@ ri_Check_Pk_Match(Relation pk_rel, Relation fk_rel, */ initStringInfo(&querybuf); quoteRelationName(pkrelname, pk_rel); - appendStringInfo(&querybuf, "SELECT 1 FROM ONLY %s x", pkrelname); + appendStringInfo(&querybuf, "SELECT 1 FROM %s x", pkrelname); querysep = "WHERE"; for (i = 0; i < riinfo->nkeys; i++) { @@ -793,7 +793,7 @@ ri_restrict(TriggerData *trigdata, bool is_no_action) /* ---------- * The query string built is - * SELECT 1 FROM ONLY <fktable> x WHERE $1 = fkatt1 [AND ...] + * SELECT 1 FROM <fktable> x WHERE $1 = fkatt1 [AND ...] * FOR KEY SHARE OF x * The type id's for the $ parameters are those of the * corresponding PK attributes. @@ -801,7 +801,7 @@ ri_restrict(TriggerData *trigdata, bool is_no_action) */ initStringInfo(&querybuf); quoteRelationName(fkrelname, fk_rel); - appendStringInfo(&querybuf, "SELECT 1 FROM ONLY %s x", + appendStringInfo(&querybuf, "SELECT 1 FROM %s x", fkrelname); querysep = "WHERE"; for (i = 0; i < riinfo->nkeys; i++) @@ -951,14 +951,14 @@ RI_FKey_cascade_del(PG_FUNCTION_ARGS) /* ---------- * The query string built is - * DELETE FROM ONLY <fktable> WHERE $1 = fkatt1 [AND ...] + * DELETE FROM <fktable> WHERE $1 = fkatt1 [AND ...] * The type id's for the $ parameters are those of the * corresponding PK attributes. * ---------- */ initStringInfo(&querybuf); quoteRelationName(fkrelname, fk_rel); - appendStringInfo(&querybuf, "DELETE FROM ONLY %s", fkrelname); + appendStringInfo(&querybuf, "DELETE FROM %s", fkrelname); querysep = "WHERE"; for (i = 0; i < riinfo->nkeys; i++) { @@ -1122,7 +1122,7 @@ RI_FKey_cascade_upd(PG_FUNCTION_ARGS) /* ---------- * The query string built is - * UPDATE ONLY <fktable> SET fkatt1 = $1 [, ...] + * UPDATE <fktable> SET fkatt1 = $1 [, ...] * WHERE $n = fkatt1 [AND ...] * The type id's for the $ parameters are those of the * corresponding PK attributes. Note that we are assuming @@ -1133,7 +1133,7 @@ RI_FKey_cascade_upd(PG_FUNCTION_ARGS) initStringInfo(&querybuf); initStringInfo(&qualbuf); quoteRelationName(fkrelname, fk_rel); - appendStringInfo(&querybuf, "UPDATE ONLY %s SET", fkrelname); + appendStringInfo(&querybuf, "UPDATE %s SET", fkrelname); querysep = ""; qualsep = "WHERE"; for (i = 0, j = riinfo->nkeys; i < riinfo->nkeys; i++, j++) @@ -1342,7 +1342,7 @@ ri_setnull(TriggerData *trigdata) /* ---------- * The query string built is - * UPDATE ONLY <fktable> SET fkatt1 = NULL [, ...] + * UPDATE <fktable> SET fkatt1 = NULL [, ...] * WHERE $1 = fkatt1 [AND ...] * The type id's for the $ parameters are those of the * corresponding PK attributes. @@ -1351,7 +1351,7 @@ ri_setnull(TriggerData *trigdata) initStringInfo(&querybuf); initStringInfo(&qualbuf); quoteRelationName(fkrelname, fk_rel); - appendStringInfo(&querybuf, "UPDATE ONLY %s SET", fkrelname); + appendStringInfo(&querybuf, "UPDATE %s SET", fkrelname); querysep = ""; qualsep = "WHERE"; for (i = 0; i < riinfo->nkeys; i++) @@ -1559,7 +1559,7 @@ ri_setdefault(TriggerData *trigdata) /* ---------- * The query string built is - * UPDATE ONLY <fktable> SET fkatt1 = DEFAULT [, ...] + * UPDATE <fktable> SET fkatt1 = DEFAULT [, ...] * WHERE $1 = fkatt1 [AND ...] * The type id's for the $ parameters are those of the * corresponding PK attributes. @@ -1568,7 +1568,7 @@ ri_setdefault(TriggerData *trigdata) initStringInfo(&querybuf); initStringInfo(&qualbuf); quoteRelationName(fkrelname, fk_rel); - appendStringInfo(&querybuf, "UPDATE ONLY %s SET", fkrelname); + appendStringInfo(&querybuf, "UPDATE %s SET", fkrelname); querysep = ""; qualsep = "WHERE"; for (i = 0; i < riinfo->nkeys; i++) @@ -1895,8 +1895,8 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel) /*---------- * The query string built is: - * SELECT fk.keycols FROM ONLY relname fk - * LEFT OUTER JOIN ONLY pkrelname pk + * SELECT fk.keycols FROM relname fk + * LEFT OUTER JOIN pkrelname pk * ON (pk.pkkeycol1=fk.keycol1 [AND ...]) * WHERE pk.pkkeycol1 IS NULL AND * For MATCH SIMPLE: @@ -1922,7 +1922,7 @@ RI_Initial_Check(Trigger *trigger, Relation fk_rel, Relation pk_rel) quoteRelationName(pkrelname, pk_rel); quoteRelationName(fkrelname, fk_rel); appendStringInfo(&querybuf, - " FROM ONLY %s fk LEFT OUTER JOIN ONLY %s pk ON", + " FROM %s fk LEFT OUTER JOIN %s pk ON", fkrelname, pkrelname); strcpy(pkattname, "pk."); @@ -2345,22 +2345,6 @@ ri_FetchConstraintInfo(Trigger *trigger, Relation trig_rel, bool rel_is_pk) /* Find or create a hashtable entry for the constraint */ riinfo = ri_LoadConstraintInfo(constraintOid); - /* Do some easy cross-checks against the trigger call data */ - if (rel_is_pk) - { - if (riinfo->fk_relid != trigger->tgconstrrelid || - riinfo->pk_relid != RelationGetRelid(trig_rel)) - elog(ERROR, "wrong pg_constraint entry for trigger \"%s\" on table \"%s\"", - trigger->tgname, RelationGetRelationName(trig_rel)); - } - else - { - if (riinfo->fk_relid != RelationGetRelid(trig_rel) || - riinfo->pk_relid != trigger->tgconstrrelid) - elog(ERROR, "wrong pg_constraint entry for trigger \"%s\" on table \"%s\"", - trigger->tgname, RelationGetRelationName(trig_rel)); - } - return riinfo; } diff --git a/src/include/catalog/pg_constraint_fn.h b/src/include/catalog/pg_constraint_fn.h index 37b0b4ba82..438f5402fe 100644 --- a/src/include/catalog/pg_constraint_fn.h +++ b/src/include/catalog/pg_constraint_fn.h @@ -56,6 +56,8 @@ extern Oid CreateConstraintEntry(const char *constraintName, bool conNoInherit, bool is_internal); +extern void CloneForeignKeyConstraints(Oid parentId, Oid relationId); + extern void RemoveConstraintById(Oid conId); extern void RenameConstraintById(Oid conId, const char *newname); extern void SetValidatedConstraintById(Oid conId); diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h index da3ff5dbee..c68515345b 100644 --- a/src/include/commands/tablecmds.h +++ b/src/include/commands/tablecmds.h @@ -74,6 +74,10 @@ extern void find_composite_type_dependencies(Oid typeOid, extern void check_of_type(HeapTuple typetuple); +extern void createForeignKeyTriggers(Relation rel, Oid refRelOid, + Constraint *fkconstraint, Oid constraintOid, + Oid indexOid); + extern void register_on_commit_action(Oid relid, OnCommitAction action); extern void remove_on_commit_action(Oid relid); diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 2caf930242..d4602d62ee 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -470,6 +470,117 @@ DROP TABLE tmp5; DROP TABLE tmp4; DROP TABLE tmp3; DROP TABLE tmp2; +-- Ensure we can add foreign keys to and from partitioned tables +SET search_path TO at_tst; +CREATE SCHEMA at_tst; +CREATE TABLE at_regular1 (col1 INT PRIMARY KEY); +CREATE TABLE at_partitioned (col2 INT PRIMARY KEY, + reg1_col1 INT NOT NULL) PARTITION BY RANGE (col2); +CREATE TABLE at_regular2 (col3 INT); +ALTER TABLE at_regular2 ADD FOREIGN KEY (col3) REFERENCES at_partitioned; +ALTER TABLE at_partitioned ADD FOREIGN KEY (reg1_col1) REFERENCES at_regular1; +CREATE TABLE at_partitioned_0 PARTITION OF at_partitioned + FOR VALUES FROM (0) TO (10000); +-- these fail: +INSERT INTO at_regular2 VALUES (1000); +ERROR: insert or update on table "at_regular2" violates foreign key constraint "at_regular2_col3_fkey" +DETAIL: Key (col3)=(1000) is not present in table "at_partitioned". +INSERT INTO at_partitioned VALUES (1000, 42); +ERROR: insert or update on table "at_partitioned_0" violates foreign key constraint "at_partitioned_reg1_col1_fkey" +DETAIL: Key (reg1_col1)=(42) is not present in table "at_regular1". +-- these work: +INSERT INTO at_regular1 VALUES (1000); +INSERT INTO at_partitioned VALUES (42, 1000); +INSERT INTO at_regular2 VALUES (42); +CREATE TABLE at_partitioned_1 PARTITION OF at_partitioned + FOR VALUES FROM (10000) TO (20000); +CREATE TABLE at_partitioned_2 (reg1_col1 INT, col2 INT); +ALTER TABLE at_partitioned ATTACH PARTITION at_partitioned_2 + FOR VALUES FROM (20000) TO (30000); +ERROR: column "col2" in child table must be marked NOT NULL +ALTER TABLE at_partitioned_2 + ALTER col2 SET NOT NULL, + ALTER reg1_col1 SET NOT NULL; +ALTER TABLE at_partitioned ATTACH PARTITION at_partitioned_2 + FOR VALUES FROM (20000) TO (30000); +\d at_regular2 + Table "at_tst.at_regular2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + col3 | integer | | | +Foreign-key constraints: + "at_regular2_col3_fkey" FOREIGN KEY (col3) REFERENCES at_partitioned(col2) + +\d at_partitioned + Table "at_tst.at_partitioned" + Column | Type | Collation | Nullable | Default +-----------+---------+-----------+----------+--------- + col2 | integer | | not null | + reg1_col1 | integer | | not null | +Partition key: RANGE (col2) +Indexes: + "at_partitioned_pkey" PRIMARY KEY, btree (col2) +Foreign-key constraints: + "at_partitioned_reg1_col1_fkey" FOREIGN KEY (reg1_col1) REFERENCES at_regular1(col1) +Referenced by: + TABLE "at_regular2" CONSTRAINT "at_regular2_col3_fkey" FOREIGN KEY (col3) REFERENCES at_partitioned(col2) +Number of partitions: 3 (Use \d+ to list them.) + +\d at_partitioned_0 + Table "at_tst.at_partitioned_0" + Column | Type | Collation | Nullable | Default +-----------+---------+-----------+----------+--------- + col2 | integer | | not null | + reg1_col1 | integer | | not null | +Partition of: at_partitioned FOR VALUES FROM (0) TO (10000) +Indexes: + "at_partitioned_0_pkey" PRIMARY KEY, btree (col2) +Foreign-key constraints: + "at_partitioned_reg1_col1_fkey" FOREIGN KEY (reg1_col1) REFERENCES at_regular1(col1) + +\d at_partitioned_1 + Table "at_tst.at_partitioned_1" + Column | Type | Collation | Nullable | Default +-----------+---------+-----------+----------+--------- + col2 | integer | | not null | + reg1_col1 | integer | | not null | +Partition of: at_partitioned FOR VALUES FROM (10000) TO (20000) +Indexes: + "at_partitioned_1_pkey" PRIMARY KEY, btree (col2) +Foreign-key constraints: + "at_partitioned_reg1_col1_fkey" FOREIGN KEY (reg1_col1) REFERENCES at_regular1(col1) + +\d at_partitioned_2 + Table "at_tst.at_partitioned_2" + Column | Type | Collation | Nullable | Default +-----------+---------+-----------+----------+--------- + reg1_col1 | integer | | not null | + col2 | integer | | not null | +Partition of: at_partitioned FOR VALUES FROM (20000) TO (30000) +Indexes: + "at_partitioned_2_pkey" PRIMARY KEY, btree (col2) + +INSERT INTO at_partitioned VALUES (5000, 42); +ERROR: insert or update on table "at_partitioned_0" violates foreign key constraint "at_partitioned_reg1_col1_fkey" +DETAIL: Key (reg1_col1)=(42) is not present in table "at_regular1". +INSERT INTO at_regular1 VALUES (42), (1042), (2042); +INSERT INTO at_partitioned VALUES (5000, 42), (15000, 1042), (25000, 2042); +INSERT INTO at_regular2 VALUES (5000), (15000), (25000); +INSERT INTO at_regular2 VALUES (35000); +ERROR: insert or update on table "at_regular2" violates foreign key constraint "at_regular2_col3_fkey" +DETAIL: Key (col3)=(35000) is not present in table "at_partitioned". +-- ok +ALTER TABLE at_regular2 DROP CONSTRAINT at_regular2_col3_fkey; +-- disallowed: must drop it from parent instead +ALTER TABLE at_partitioned_0 DROP CONSTRAINT at_partitioned_reg1_col1_fkey; +ERROR: cannot drop inherited constraint "at_partitioned_reg1_col1_fkey" of relation "at_partitioned_0" +-- ok +ALTER TABLE at_partitioned DROP CONSTRAINT at_partitioned_reg1_col1_fkey; +\set VERBOSITY terse +DROP SCHEMA at_tst CASCADE; +NOTICE: drop cascades to 3 other objects +\set VERBOSITY default +RESET search_path; -- NOT VALID with plan invalidation -- ensure we don't use a constraint for -- exclusion until validated set constraint_exclusion TO 'partition'; @@ -3290,10 +3401,6 @@ CREATE TABLE partitioned ( a int, b int ) PARTITION BY RANGE (a, (a+b+1)); -ALTER TABLE partitioned ADD FOREIGN KEY (a) REFERENCES blah; -ERROR: foreign key constraints are not supported on partitioned tables -LINE 1: ALTER TABLE partitioned ADD FOREIGN KEY (a) REFERENCES blah; - ^ ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&); ERROR: exclusion constraints are not supported on partitioned tables LINE 1: ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&); diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out index 866cc99b9f..7c3703b73d 100644 --- a/src/test/regress/expected/create_table.out +++ b/src/test/regress/expected/create_table.out @@ -276,16 +276,6 @@ CREATE TABLE partitioned ( ) PARTITION BY LIST (a1, a2); -- fail ERROR: cannot use "list" partition strategy with more than one column -- unsupported constraint type for partitioned tables -CREATE TABLE pkrel ( - a int PRIMARY KEY -); -CREATE TABLE partitioned ( - a int REFERENCES pkrel(a) -) PARTITION BY RANGE (a); -ERROR: foreign key constraints are not supported on partitioned tables -LINE 2: a int REFERENCES pkrel(a) - ^ -DROP TABLE pkrel; CREATE TABLE partitioned ( a int, EXCLUDE USING gist (a WITH &&) diff --git a/src/test/regress/expected/foreign_key.out b/src/test/regress/expected/foreign_key.out index fef072eddf..8c71c21973 100644 --- a/src/test/regress/expected/foreign_key.out +++ b/src/test/regress/expected/foreign_key.out @@ -1415,3 +1415,70 @@ alter table fktable2 drop constraint fktable2_f1_fkey; ERROR: cannot ALTER TABLE "pktable2" because it has pending trigger events commit; drop table pktable2, fktable2; +-- +-- Foreign keys and partitioned tables +-- +-- Test that it's possible to have a FK from a partitioned table to a regular +-- one +CREATE TABLE pkregular (f1 int primary key); +CREATE TABLE fkpartit (f1 int references pkregular) PARTITION BY RANGE (f1); +CREATE TABLE fkpart1 PARTITION OF fkpartit FOR VALUES FROM (0) TO (1000); +INSERT INTO fkpartit VALUES (500); +ERROR: insert or update on table "fkpart1" violates foreign key constraint "fkpartit_f1_fkey" +DETAIL: Key (f1)=(500) is not present in table "pkregular". +INSERT INTO fkpart1 VALUES (500); +ERROR: insert or update on table "fkpart1" violates foreign key constraint "fkpartit_f1_fkey" +DETAIL: Key (f1)=(500) is not present in table "pkregular". +INSERT INTO pkregular VALUES (500); +INSERT INTO fkpartit VALUES (500); +INSERT INTO fkpart1 VALUES (500); +DELETE FROM pkregular; +ERROR: update or delete on table "pkregular" violates foreign key constraint "fkpartit_f1_fkey" on table "fkpartit" +DETAIL: Key (f1)=(500) is still referenced from table "fkpartit". +UPDATE pkregular SET f1 = 501; +ERROR: update or delete on table "pkregular" violates foreign key constraint "fkpartit_f1_fkey" on table "fkpartit" +DETAIL: Key (f1)=(500) is still referenced from table "fkpartit". +ALTER TABLE fkpart1 DROP CONSTRAINT fkpartit_f1_fkey; -- nope +ERROR: cannot drop inherited constraint "fkpartit_f1_fkey" of relation "fkpart1" +ALTER TABLE fkpartit DROP CONSTRAINT fkpartit_f1_fkey; +\d fkpartit + Table "public.fkpartit" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + f1 | integer | | | +Partition key: RANGE (f1) +Number of partitions: 1 (Use \d+ to list them.) + +\d fkpart1 + Table "public.fkpart1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + f1 | integer | | | +Partition of: fkpartit FOR VALUES FROM (0) TO (1000) + +ALTER TABLE fkpartit ADD CONSTRAINT fkpartit_f1_fkey FOREIGN KEY (f1) REFERENCES pkregular ON DELETE CASCADE; +\d fkpartit + Table "public.fkpartit" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + f1 | integer | | | +Partition key: RANGE (f1) +Foreign-key constraints: + "fkpartit_f1_fkey" FOREIGN KEY (f1) REFERENCES pkregular(f1) ON DELETE CASCADE +Number of partitions: 1 (Use \d+ to list them.) + +\d fkpart1 + Table "public.fkpart1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + f1 | integer | | | +Partition of: fkpartit FOR VALUES FROM (0) TO (1000) +Foreign-key constraints: + "fkpartit_f1_fkey" FOREIGN KEY (f1) REFERENCES pkregular(f1) ON DELETE CASCADE + +DELETE FROM pkregular; +SELECT * FROM fkpartit; + f1 +---- +(0 rows) + diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index ed0bb7845b..40b3d0256b 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -375,6 +375,62 @@ DROP TABLE tmp3; DROP TABLE tmp2; +-- Ensure we can add foreign keys to and from partitioned tables +SET search_path TO at_tst; +CREATE SCHEMA at_tst; +CREATE TABLE at_regular1 (col1 INT PRIMARY KEY); +CREATE TABLE at_partitioned (col2 INT PRIMARY KEY, + reg1_col1 INT NOT NULL) PARTITION BY RANGE (col2); +CREATE TABLE at_regular2 (col3 INT); +ALTER TABLE at_regular2 ADD FOREIGN KEY (col3) REFERENCES at_partitioned; +ALTER TABLE at_partitioned ADD FOREIGN KEY (reg1_col1) REFERENCES at_regular1; +CREATE TABLE at_partitioned_0 PARTITION OF at_partitioned + FOR VALUES FROM (0) TO (10000); +-- these fail: +INSERT INTO at_regular2 VALUES (1000); +INSERT INTO at_partitioned VALUES (1000, 42); + +-- these work: +INSERT INTO at_regular1 VALUES (1000); +INSERT INTO at_partitioned VALUES (42, 1000); +INSERT INTO at_regular2 VALUES (42); + +CREATE TABLE at_partitioned_1 PARTITION OF at_partitioned + FOR VALUES FROM (10000) TO (20000); +CREATE TABLE at_partitioned_2 (reg1_col1 INT, col2 INT); +ALTER TABLE at_partitioned ATTACH PARTITION at_partitioned_2 + FOR VALUES FROM (20000) TO (30000); +ALTER TABLE at_partitioned_2 + ALTER col2 SET NOT NULL, + ALTER reg1_col1 SET NOT NULL; +ALTER TABLE at_partitioned ATTACH PARTITION at_partitioned_2 + FOR VALUES FROM (20000) TO (30000); + +\d at_regular2 +\d at_partitioned +\d at_partitioned_0 +\d at_partitioned_1 +\d at_partitioned_2 + +INSERT INTO at_partitioned VALUES (5000, 42); +INSERT INTO at_regular1 VALUES (42), (1042), (2042); +INSERT INTO at_partitioned VALUES (5000, 42), (15000, 1042), (25000, 2042); +INSERT INTO at_regular2 VALUES (5000), (15000), (25000); +INSERT INTO at_regular2 VALUES (35000); + +-- ok +ALTER TABLE at_regular2 DROP CONSTRAINT at_regular2_col3_fkey; + +-- disallowed: must drop it from parent instead +ALTER TABLE at_partitioned_0 DROP CONSTRAINT at_partitioned_reg1_col1_fkey; +-- ok +ALTER TABLE at_partitioned DROP CONSTRAINT at_partitioned_reg1_col1_fkey; + +\set VERBOSITY terse +DROP SCHEMA at_tst CASCADE; +\set VERBOSITY default +RESET search_path; + -- NOT VALID with plan invalidation -- ensure we don't use a constraint for -- exclusion until validated set constraint_exclusion TO 'partition'; @@ -2016,7 +2072,6 @@ CREATE TABLE partitioned ( a int, b int ) PARTITION BY RANGE (a, (a+b+1)); -ALTER TABLE partitioned ADD FOREIGN KEY (a) REFERENCES blah; ALTER TABLE partitioned ADD EXCLUDE USING gist (a WITH &&); -- cannot drop column that is part of the partition key diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql index fefccf21a2..09a634d79d 100644 --- a/src/test/regress/sql/create_table.sql +++ b/src/test/regress/sql/create_table.sql @@ -294,14 +294,6 @@ CREATE TABLE partitioned ( ) PARTITION BY LIST (a1, a2); -- fail -- unsupported constraint type for partitioned tables -CREATE TABLE pkrel ( - a int PRIMARY KEY -); -CREATE TABLE partitioned ( - a int REFERENCES pkrel(a) -) PARTITION BY RANGE (a); -DROP TABLE pkrel; - CREATE TABLE partitioned ( a int, EXCLUDE USING gist (a WITH &&) diff --git a/src/test/regress/sql/foreign_key.sql b/src/test/regress/sql/foreign_key.sql index 5f19dad03c..1a2c93fcce 100644 --- a/src/test/regress/sql/foreign_key.sql +++ b/src/test/regress/sql/foreign_key.sql @@ -1055,3 +1055,31 @@ alter table fktable2 drop constraint fktable2_f1_fkey; commit; drop table pktable2, fktable2; + + +-- +-- Foreign keys and partitioned tables +-- + +-- Test that it's possible to have a FK from a partitioned table to a regular +-- one +CREATE TABLE pkregular (f1 int primary key); +CREATE TABLE fkpartit (f1 int references pkregular) PARTITION BY RANGE (f1); +CREATE TABLE fkpart1 PARTITION OF fkpartit FOR VALUES FROM (0) TO (1000); +INSERT INTO fkpartit VALUES (500); +INSERT INTO fkpart1 VALUES (500); +INSERT INTO pkregular VALUES (500); +INSERT INTO fkpartit VALUES (500); +INSERT INTO fkpart1 VALUES (500); +DELETE FROM pkregular; +UPDATE pkregular SET f1 = 501; + +ALTER TABLE fkpart1 DROP CONSTRAINT fkpartit_f1_fkey; -- nope +ALTER TABLE fkpartit DROP CONSTRAINT fkpartit_f1_fkey; +\d fkpartit +\d fkpart1 +ALTER TABLE fkpartit ADD CONSTRAINT fkpartit_f1_fkey FOREIGN KEY (f1) REFERENCES pkregular ON DELETE CASCADE; +\d fkpartit +\d fkpart1 +DELETE FROM pkregular; +SELECT * FROM fkpartit; -- 2.11.0

view thread (23+ messages)  latest in thread

Message-ID: <20171231194908.qbtfvrt77lgda4xf@alvherre.pgsql>
Permalink:  ../20171231194908.qbtfvrt77lgda4xf@alvherre.pgsql/
Also on:    postgresql.org/message-id/20171231194908.qbtfvrt77lgda4xf@alvherre.pgsql

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: alvherre@alvh.no-ip.org
  Subject: Re: Foreign keys and partitioned tables
  In-Reply-To: <20171231194908.qbtfvrt77lgda4xf@alvherre.pgsql>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox