Michael, * Stephen Frost (sfr...@snowman.net) wrote: > * Michael Paquier (michael.paqu...@gmail.com) wrote: > > The thing is that we must absolutely wait for *all* the TableInfoData > > of all the extensions to be created because we need to mark the > > dependencies between them, and even my last patch, or even with what > > you are proposing we may miss tracking of FK dependencies between > > tables in different extensions. This has little chance to happen in > > practice, but I think that we should definitely get things right. > > Hence something like this query able to query all the FK dependencies > > with tables in extensions is more useful, and it has no IN clause: > > Ah, yes, extensions can depend on each other and so this could > definitely happen. The current issue is around postgis, which by itself > provides three different extensions. > > > + appendPQExpBuffer(query, > > + "SELECT conrelid, confrelid " > > + "FROM pg_constraint " > > + "LEFT JOIN pg_depend ON (objid = confrelid) " > > + "WHERE contype = 'f' " > > + "AND refclassid = 'pg_extension'::regclass " > > + "AND classid = 'pg_class'::regclass;"); > > I'm trying to figure out why this is a left join..?
Please see the latest version of this, attached. I've removed the left
join, re-used the 'query' buffer (instead of destroying and recreating
it), and added a bit of documentation, along with a note in the commit
message for the release notes.
Would be great if you could review it and let me know. As mentioned in
my other email, I'm happy to include the TAP test in master once we've
worked out the correct way to handle installing the extension for
testing in the Makefile system.
Thanks!
Stephen
From ffc459240a97bc4e7a4e4bc81be7a019d4f6782e Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfr...@snowman.net>
Date: Sun, 1 Mar 2015 15:51:04 -0500
Subject: [PATCH] Fix pg_dump handling of extension config tables
Since 9.1, we've provided extensions with a way to denote
"configuration" tables- tables created by an extension which the user
may modify. By marking these as "configuration" tables, the extension
is asking for the data in these tables to be pg_dump'd (tables which
are not marked in this way are assumed to be entirely handled during
CREATE EXTENSION and are not included at all in a pg_dump).
Unfortunately, pg_dump neglected to consider foreign key relationships
between extension configuration tables and therefore could end up
trying to reload the data in an order which would cause FK violations.
This patch teaches pg_dump about these dependencies, so that the data
dumped out is done so in the best order possible. Note that there's no
way to handle circular dependencies, but those have yet to be seen in
the wild.
The release notes for this should include a caution to users that
existing pg_dump-based backups may be invalid due to this issue. The
data is all there, but restoring from it will require extracting the
data for the configuration tables and then loading them in the correct
order by hand.
Discussed initially back in bug #6738, more recently brought up by
Gilles Darold, who provided an initial patch which was further reworked
by Michael Paquier. Further modifications and documentation updates
by me.
Back-patch to 9.1 where we added the concept of extension configuration
tables.
---
doc/src/sgml/extend.sgml | 8 +++++++
src/bin/pg_dump/pg_dump.c | 54 +++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index be10252..0a79f56 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -721,6 +721,14 @@ SELECT pg_catalog.pg_extension_config_dump('my_config', 'WHERE NOT standard_entr
a table as no longer a configuration table is to dissociate it from the
extension with <command>ALTER EXTENSION ... DROP TABLE</>.
</para>
+
+ <para>
+ Note that foreign key relationships between these tables will dictate the
+ order in which the tables are dumped out by pg_dump. Specifically, it will
+ attempt to dump the referenced-by table before the referencing table. As
+ the foreign key relationships are set up at CREATE EXTENSION time (prior to
+ data being loaded into the tables) circular dependencies are not supported.
+ </para>
</sect2>
<sect2>
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 2b53c72..4e404b4 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -15236,7 +15236,8 @@ dumpRule(Archive *fout, DumpOptions *dopt, RuleInfo *rinfo)
}
/*
- * getExtensionMembership --- obtain extension membership data
+ * getExtensionMembership --- obtain extension membership data and check FK
+ * dependencies among relations interacting with the ones in extensions.
*/
void
getExtensionMembership(Archive *fout, DumpOptions *dopt, ExtensionInfo extinfo[],
@@ -15249,7 +15250,9 @@ getExtensionMembership(Archive *fout, DumpOptions *dopt, ExtensionInfo extinfo[]
int i_classid,
i_objid,
i_refclassid,
- i_refobjid;
+ i_refobjid,
+ i_conrelid,
+ i_confrelid;
DumpableObject *dobj,
*refdobj;
@@ -15430,6 +15433,53 @@ getExtensionMembership(Archive *fout, DumpOptions *dopt, ExtensionInfo extinfo[]
free(extconditionarray);
}
+ /*
+ * Now that all the TableInfoData objects have been created for all
+ * the extensions, check their FK dependencies and register them to
+ * ensure correct data ordering. Note that this is not a problem
+ * for user tables not included in an extension referencing with a
+ * FK tables in extensions as their constraint is declared after
+ * dumping their data. In --data-only mode the table ordering is
+ * ensured as well thanks to getTableDataFKConstraints().
+ */
+ printfPQExpBuffer(query,
+ "SELECT conrelid, confrelid "
+ "FROM pg_constraint "
+ "JOIN pg_depend ON (objid = confrelid) "
+ "WHERE contype = 'f' "
+ "AND refclassid = 'pg_extension'::regclass "
+ "AND classid = 'pg_class'::regclass;");
+
+ res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+ ntups = PQntuples(res);
+
+ i_conrelid = PQfnumber(res, "conrelid");
+ i_confrelid = PQfnumber(res, "confrelid");
+
+ /* Now get the dependencies and register them */
+ for (i = 0; i < ntups; i++)
+ {
+ Oid conrelid, confrelid;
+ TableInfo *reftable, *contable;
+
+ conrelid = atooid(PQgetvalue(res, i, i_conrelid));
+ confrelid = atooid(PQgetvalue(res, i, i_confrelid));
+ contable = findTableByOid(conrelid);
+ reftable = findTableByOid(confrelid);
+
+ if (reftable == NULL ||
+ reftable->dataObj == NULL ||
+ contable == NULL ||
+ contable->dataObj == NULL)
+ continue;
+
+ /*
+ * Make referencing TABLE_DATA object depend on the
+ * referenced table's TABLE_DATA object.
+ */
+ addObjectDependency(&contable->dataObj->dobj,
+ reftable->dataObj->dobj.dumpId);
+ }
destroyPQExpBuffer(query);
}
--
1.9.1
signature.asc
Description: Digital signature
