Hello,

There's a long pending issue with pg_dump and extensions that have table
members with foreign keys. This was previously reported in this thread
http://www.postgresql.org/message-id/ca+tgmoyvzkadmgh_8el7uvm472geru0b4pnnfjqye6ss1k9...@mail.gmail.com
and discuss by Robert. All PostgreSQL users that use the PostGis
extension postgis_topology are facing the issue because the two members
tables (topology and layer) are linked by foreign keys.

If you dump a database with this extension and try to import it you will
experience this error:

    pg_restore: [archiver (db)] Error while PROCESSING TOC:
    pg_restore: [archiver (db)] Error from TOC entry 3345; 0 157059176
    TABLE DATA layer gilles
    pg_restore: [archiver (db)] COPY failed for table "layer": ERROR: 
    insert or update on table "layer" violates foreign key constraint
    "layer_topology_id_fkey"
    DETAIL:  Key (topology_id)=(1) is not present in table "topology".
    WARNING: errors ignored on restore: 1


The problem is that, whatever export type you choose (plain/custom and
full-export/data-only) the data of tables "topology" and "layer" are
always exported in alphabetic order. I think this is a bug because
outside extension, in data-only export, pg_dump is able to find foreign
keys dependency and dump table's data in the right order but not with
extension's members. Default is alphabetic order but that should not be
the case with extension's members because constraints are recreated
during the CREATE EXTENSION order. I hope I am clear enough.

Here we have three solutions:

    1/ Inform developers of extensions to take care to alphabetical
order when they have member tables using foreign keys.
    2/ Inform DBAs that they have to restore the failing table
independently. The use case above can be resumed using the following
command:

         pg_restore -h localhost -n topology -t layer -Fc -d
testdb_empty testdump.dump

    3/ Inform DBAs that they have to restore the schema first then the
data only using --disable-triggers
    4/ Patch pg_dump to solve this issue.

I attach a patch that solves the issue in pg_dump, let me know if it
might be included in Commit Fest or if the three other solutions are a
better choice. I also join a sample extension (test_fk_in_ext) to be
able to reproduce the issue and test the patch. Note that it might
exists a simpler solution than the one I used in this patch, if this is
the case please point me on the right way, I will be pleased to rewrite
and send an other patch.

In the test extension attached, there is a file called
test_fk_in_ext/SYNOPSIS.txt that describe all actions to reproduce the
issue and test the patch. Here is the SQL part of the test extension:

    CREATE TABLE IF NOT EXISTS b_test_fk_in_ext1 (
            id int PRIMARY KEY
    );

    CREATE TABLE IF NOT EXISTS a_test_fk_in_ext1 (
            id int REFERENCES b_test_fk_in_ext1(id)
    );

    SELECT pg_catalog.pg_extension_config_dump('b_test_fk_in_ext1', '');
    SELECT pg_catalog.pg_extension_config_dump('a_test_fk_in_ext1', '');



Best regards,

-- 
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index dc062e6..49889ce 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -209,6 +209,7 @@ static void addBoundaryDependencies(DumpableObject **dobjs, int numObjs,
 
 static void getDomainConstraints(Archive *fout, TypeInfo *tyinfo);
 static void getTableData(DumpOptions *dopt, TableInfo *tblinfo, int numTables, bool oids);
+static bool hasExtensionMember(TableInfo *tblinfo, int numTables);
 static void makeTableDataInfo(DumpOptions *dopt, TableInfo *tbinfo, bool oids);
 static void buildMatViewRefreshDependencies(Archive *fout);
 static void getTableDataFKConstraints(void);
@@ -730,9 +731,17 @@ main(int argc, char **argv)
 
 	if (!dopt.schemaOnly)
 	{
+		bool has_ext_member;
+
 		getTableData(&dopt, tblinfo, numTables, dopt.oids);
+		/* Search if there is dumpable tables member of and extension */
+		has_ext_member = hasExtensionMember(tblinfo, numTables);
 		buildMatViewRefreshDependencies(fout);
-		if (dopt.dataOnly)
+		/*
+		 * Always get FK constraints even with schema+data, extension's
+		 * members can have FK so tables need to be dump-ordered.
+		 */
+		if (dopt.dataOnly || has_ext_member)
 			getTableDataFKConstraints();
 	}
 
@@ -1852,6 +1861,25 @@ getTableData(DumpOptions *dopt, TableInfo *tblinfo, int numTables, bool oids)
 }
 
 /*
+ * hasExtensionMember -
+ *	  set up dumpable objects representing the contents of tables
+ */
+static bool
+hasExtensionMember(TableInfo *tblinfo, int numTables)
+{
+	int			i;
+
+	for (i = 0; i < numTables; i++)
+	{
+		if (tblinfo[i].dobj.ext_member)
+			return true;
+	}
+
+	return false;
+}
+
+
+/*
  * Make a dumpable object for the data of this specific table
  *
  * Note: we make a TableDataInfo if and only if we are going to dump the
@@ -2024,12 +2052,14 @@ buildMatViewRefreshDependencies(Archive *fout)
 
 /*
  * getTableDataFKConstraints -
- *	  add dump-order dependencies reflecting foreign key constraints
+ *        add dump-order dependencies reflecting foreign key constraints
  *
- * This code is executed only in a data-only dump --- in schema+data dumps
- * we handle foreign key issues by not creating the FK constraints until
- * after the data is loaded.  In a data-only dump, however, we want to
- * order the table data objects in such a way that a table's referenced
+ * This code is executed only in a data-only dump or when there is extension's
+ * member -- in schema+data dumps we handle foreign key issues by not creating
+ * the FK constraints until after the data is loaded. In a data-only dump or
+ * when there is an extension member to dump (schema dumps do not concern
+ * extension's objects, they are created during the CREATE EXTENSION), we want
+ * to order the table data objects in such a way that a table's referenced
  * tables are restored first.  (In the presence of circular references or
  * self-references this may be impossible; we'll detect and complain about
  * that during the dependency sorting step.)
@@ -2929,6 +2959,10 @@ dumpPolicy(Archive *fout, DumpOptions *dopt, PolicyInfo *polinfo)
 	if (dopt->dataOnly)
 		return;
 
+	/* Do not dump if table is member of an extension */
+	if (tbinfo->dobj.ext_member)
+		return;
+
 	/*
 	 * If polname is NULL, then this record is just indicating that ROW
 	 * LEVEL SECURITY is enabled for the table. Dump as ALTER TABLE <table>
@@ -7880,6 +7914,10 @@ dumpTableComment(Archive *fout, DumpOptions *dopt, TableInfo *tbinfo,
 	if (dopt->dataOnly)
 		return;
 
+	/* Do not dump if table is member of an extension */
+	if (tbinfo->dobj.ext_member)
+		return;
+
 	/* Search for comments associated with relation, using table */
 	ncomments = findComments(fout,
 							 tbinfo->dobj.catId.tableoid,
@@ -13134,6 +13172,10 @@ dumpTableSecLabel(Archive *fout, DumpOptions *dopt, TableInfo *tbinfo, const cha
 	if (dopt->dataOnly)
 		return;
 
+	/* Do not dump if table is member of an extension */
+	if (tbinfo->dobj.ext_member)
+		return;
+
 	/* Search for comments associated with relation, using table */
 	nlabels = findSecLabels(fout,
 							tbinfo->dobj.catId.tableoid,
@@ -13341,7 +13383,7 @@ collectSecLabels(Archive *fout, SecLabelItem **items)
 static void
 dumpTable(Archive *fout, DumpOptions *dopt, TableInfo *tbinfo)
 {
-	if (tbinfo->dobj.dump && !dopt->dataOnly)
+	if (tbinfo->dobj.dump && !dopt->dataOnly && !tbinfo->dobj.ext_member)
 	{
 		char	   *namecopy;
 
@@ -13479,6 +13521,10 @@ dumpTableSchema(Archive *fout, DumpOptions *dopt, TableInfo *tbinfo)
 	int			j,
 				k;
 
+	/* Do not dump if table is member of an extension */
+	if (tbinfo->dobj.ext_member)
+		return;
+
 	/* Make sure we are in proper schema */
 	selectSourceSchema(fout, tbinfo->dobj.namespace->dobj.name);
 
@@ -14113,6 +14159,10 @@ dumpAttrDef(Archive *fout, DumpOptions *dopt, AttrDefInfo *adinfo)
 	if (!tbinfo->dobj.dump || dopt->dataOnly)
 		return;
 
+	/* Do not dump if table is member of an extension */
+	if (tbinfo->dobj.ext_member)
+		return;
+
 	/* Skip if not "separate"; it was dumped in the table's definition */
 	if (!adinfo->separate)
 		return;
@@ -14200,6 +14250,10 @@ dumpIndex(Archive *fout, DumpOptions *dopt, IndxInfo *indxinfo)
 	if (dopt->dataOnly)
 		return;
 
+	/* Do not dump if table is member of an extension */
+	if (tbinfo->dobj.ext_member)
+		return;
+
 	q = createPQExpBuffer();
 	delq = createPQExpBuffer();
 	labelq = createPQExpBuffer();
@@ -14289,6 +14343,10 @@ dumpConstraint(Archive *fout, DumpOptions *dopt, ConstraintInfo *coninfo)
 	if (!coninfo->dobj.dump || dopt->dataOnly)
 		return;
 
+	/* Do not dump if table is member of an extension */
+	if ((tbinfo != NULL) && tbinfo->dobj.ext_member)
+		return;
+
 	q = createPQExpBuffer();
 	delq = createPQExpBuffer();
 
@@ -14513,7 +14571,13 @@ static void
 dumpTableConstraintComment(Archive *fout, DumpOptions *dopt, ConstraintInfo *coninfo)
 {
 	TableInfo  *tbinfo = coninfo->contable;
-	PQExpBuffer labelq = createPQExpBuffer();
+	PQExpBuffer labelq;
+
+	/* Do not dump if table is member of an extension */
+	if (tbinfo->dobj.ext_member)
+		return;
+
+	labelq = createPQExpBuffer();
 
 	appendPQExpBuffer(labelq, "CONSTRAINT %s ",
 					  fmtId(coninfo->dobj.name));
@@ -14590,6 +14654,11 @@ dumpSequence(Archive *fout, DumpOptions *dopt, TableInfo *tbinfo)
 	char		bufm[100],
 				bufx[100];
 	bool		cycled;
+
+	/* Do not dump if table is member of an extension */
+	if (tbinfo->dobj.ext_member)
+		return;
+
 	PQExpBuffer query = createPQExpBuffer();
 	PQExpBuffer delqry = createPQExpBuffer();
 	PQExpBuffer labelq = createPQExpBuffer();
@@ -14857,6 +14926,10 @@ dumpTrigger(Archive *fout, DumpOptions *dopt, TriggerInfo *tginfo)
 	if (dopt->dataOnly)
 		return;
 
+	/* Do not dump if table is member of an extension */
+	if (tbinfo->dobj.ext_member)
+		return;
+
 	query = createPQExpBuffer();
 	delqry = createPQExpBuffer();
 	labelq = createPQExpBuffer();
@@ -15133,6 +15206,10 @@ dumpRule(Archive *fout, DumpOptions *dopt, RuleInfo *rinfo)
 	if (!rinfo->dobj.dump || dopt->dataOnly)
 		return;
 
+	/* Do not dump if table is member of an extension */
+	if (tbinfo->dobj.ext_member)
+		return;
+
 	/*
 	 * If it is an ON SELECT rule that is created implicitly by CREATE VIEW,
 	 * we do not want to dump it as a separate object.
@@ -15423,6 +15500,8 @@ getExtensionMembership(Archive *fout, DumpOptions *dopt, ExtensionInfo extinfo[]
 
 				if (dumpobj)
 				{
+					/* Mark the member table as dumpable */
+					configtbl->dobj.dump = true;
 					/*
 					 * Note: config tables are dumped without OIDs regardless
 					 * of the --oids setting.  This is because row filtering

Attachment: test_fk_in_ext.tar.gz
Description: GNU Zip compressed data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to