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