On Wed, Feb 25, 2015 at 7:46 AM, Gilles Darold <gilles.dar...@dalibo.com> wrote:
> This is a far better patch and the test to export/import of the
> postgis_topology extension works great for me.
>
> Thanks for the work.
Attached is a patch that uses an even better approach by querying only
once all the FK dependencies of tables in extensions whose data is
registered as dumpable by getExtensionMembership(). Could you check
that it works correctly? My test case passes but an extra check would
be a good nice. The patch footprint is pretty low so we may be able to
backport this patch easily.
--
Michael
From 72e369ee725301003cf065b0bf9875724455dac1 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Tue, 17 Feb 2015 07:39:23 +0000
Subject: [PATCH 1/3] Fix ordering of tables part of extensions linked with
constraints
Additional checks on FK constraints potentially linking between them
extension objects are done and data dump ordering is ensured. Note
that this does not take into account foreign keys of tables that are
not part of an extension linking to an extension table.
---
src/bin/pg_dump/pg_dump.c | 80 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 78 insertions(+), 2 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 2b53c72..bbcd600 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 extension tables.
*/
void
getExtensionMembership(Archive *fout, DumpOptions *dopt, ExtensionInfo extinfo[],
@@ -15368,7 +15369,9 @@ getExtensionMembership(Archive *fout, DumpOptions *dopt, ExtensionInfo extinfo[]
parsePGArray(extcondition, &extconditionarray, &nconditionitems) &&
nconfigitems == nconditionitems)
{
- int j;
+ int j, i_conrelid, i_confrelid;
+ PQExpBuffer query2;
+ bool first_elt = true;
for (j = 0; j < nconfigitems; j++)
{
@@ -15423,6 +15426,79 @@ getExtensionMembership(Archive *fout, DumpOptions *dopt, ExtensionInfo extinfo[]
}
}
}
+
+ /*
+ * 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().
+ */
+ query2 = createPQExpBuffer();
+
+ /*
+ * Query all the foreign key dependencies for all the extension
+ * tables found previously. Only tables whose data need to be
+ * have to be tracked.
+ */
+ appendPQExpBuffer(query2,
+ "SELECT conrelid, confrelid "
+ "FROM pg_catalog.pg_constraint "
+ "WHERE contype = 'f' AND conrelid IN (");
+
+ for (j = 0; j < nconfigitems; j++)
+ {
+ TableInfo *configtbl;
+ Oid configtbloid = atooid(extconfigarray[j]);
+
+ configtbl = findTableByOid(configtbloid);
+ if (configtbl == NULL || configtbl->dataObj == NULL)
+ continue;
+
+ if (first_elt)
+ {
+ appendPQExpBuffer(query2, "%u", configtbloid);
+ first_elt = false;
+ }
+ else
+ appendPQExpBuffer(query2, ", %u", configtbloid);
+ }
+ appendPQExpBuffer(query2, ");");
+
+ res = ExecuteSqlQuery(fout, query2->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 (j = 0; j < ntups; j++)
+ {
+ Oid conrelid, confrelid;
+ TableInfo *reftable, *contable;
+
+ conrelid = atooid(PQgetvalue(res, j, i_conrelid));
+ confrelid = atooid(PQgetvalue(res, j, 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);
+ }
+ resetPQExpBuffer(query2);
}
if (extconfigarray)
free(extconfigarray);
--
2.3.0
From 1d8fe1c39ec5049c39810f94153d347971738616 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Tue, 17 Feb 2015 22:29:28 +0900
Subject: [PATCH 2/3] Make prove_check install contents of current directory as
well
This is useful for example for TAP tests in extensions.
---
src/Makefile.global.in | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 7c39d82..7c15423 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -323,6 +323,7 @@ endef
define prove_check
$(MKDIR_P) tmp_check/log
$(MAKE) -C $(top_builddir) DESTDIR='$(CURDIR)'/tmp_check/install install >'$(CURDIR)'/tmp_check/log/install.log 2>&1
+$(MAKE) -C $(CURDIR) DESTDIR='$(CURDIR)'/tmp_check/install install >>'$(CURDIR)'/tmp_check/log/install.log 2>&1
cd $(srcdir) && TESTDIR='$(CURDIR)' PATH="$(CURDIR)/tmp_check/install$(bindir):$$PATH" $(call add_to_path,$(ld_library_path_var),$(CURDIR)/tmp_check/install$(libdir)) top_builddir='$(CURDIR)/$(top_builddir)' PGPORT='6$(DEF_PGPORT)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) t/*.pl
endef
--
2.3.0
From 3925dfac6745c7a3141901739d142678c45ce48e Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Tue, 17 Feb 2015 22:36:49 +0900
Subject: [PATCH 3/3] Add dump_test, test module to check pg_dump with
extensions
It works with TAP tests.
---
src/test/modules/Makefile | 1 +
src/test/modules/dump_test/.gitignore | 4 ++++
src/test/modules/dump_test/Makefile | 22 ++++++++++++++++++++++
src/test/modules/dump_test/README | 5 +++++
src/test/modules/dump_test/dump_test--1.0.sql | 20 ++++++++++++++++++++
src/test/modules/dump_test/dump_test.control | 5 +++++
src/test/modules/dump_test/t/001_dump_test.pl | 27 +++++++++++++++++++++++++++
7 files changed, 84 insertions(+)
create mode 100644 src/test/modules/dump_test/.gitignore
create mode 100644 src/test/modules/dump_test/Makefile
create mode 100644 src/test/modules/dump_test/README
create mode 100644 src/test/modules/dump_test/dump_test--1.0.sql
create mode 100644 src/test/modules/dump_test/dump_test.control
create mode 100644 src/test/modules/dump_test/t/001_dump_test.pl
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index 93d93af..6ad3b4e 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -6,6 +6,7 @@ include $(top_builddir)/src/Makefile.global
SUBDIRS = \
commit_ts \
+ dump_test \
worker_spi \
dummy_seclabel \
test_shm_mq \
diff --git a/src/test/modules/dump_test/.gitignore b/src/test/modules/dump_test/.gitignore
new file mode 100644
index 0000000..5dcb3ff
--- /dev/null
+++ b/src/test/modules/dump_test/.gitignore
@@ -0,0 +1,4 @@
+# Generated subdirectories
+/log/
+/results/
+/tmp_check/
diff --git a/src/test/modules/dump_test/Makefile b/src/test/modules/dump_test/Makefile
new file mode 100644
index 0000000..08cd215
--- /dev/null
+++ b/src/test/modules/dump_test/Makefile
@@ -0,0 +1,22 @@
+# src/test/modules/dump_test/Makefile
+
+EXTENSION = dump_test
+DATA = dump_test--1.0.sql
+PGFILEDESC = "dump_test - test pg_dump with extensions"
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/dump_test
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
+
+check: all
+ $(prove_check)
+
+installcheck: install
+ $(prove_installcheck)
diff --git a/src/test/modules/dump_test/README b/src/test/modules/dump_test/README
new file mode 100644
index 0000000..9ebfa19
--- /dev/null
+++ b/src/test/modules/dump_test/README
@@ -0,0 +1,5 @@
+dump_test
+=========
+
+Simple module used to test consistency of pg_dump with objects in
+extensions.
diff --git a/src/test/modules/dump_test/dump_test--1.0.sql b/src/test/modules/dump_test/dump_test--1.0.sql
new file mode 100644
index 0000000..d684855
--- /dev/null
+++ b/src/test/modules/dump_test/dump_test--1.0.sql
@@ -0,0 +1,20 @@
+/* src/test/modules/dump_test/dump_test--1.0.sql */
+
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION dump_test" to load this file. \quit
+
+CREATE TABLE IF NOT EXISTS cc_tab_fkey (
+ id int PRIMARY KEY
+);
+
+CREATE TABLE IF NOT EXISTS bb_tab_fkey (
+ id int PRIMARY KEY REFERENCES cc_tab_fkey(id)
+);
+
+CREATE TABLE IF NOT EXISTS aa_tab_fkey (
+ id int REFERENCES bb_tab_fkey(id)
+);
+
+SELECT pg_catalog.pg_extension_config_dump('aa_tab_fkey', '');
+SELECT pg_catalog.pg_extension_config_dump('bb_tab_fkey', '');
+SELECT pg_catalog.pg_extension_config_dump('cc_tab_fkey', '');
diff --git a/src/test/modules/dump_test/dump_test.control b/src/test/modules/dump_test/dump_test.control
new file mode 100644
index 0000000..b32c99c
--- /dev/null
+++ b/src/test/modules/dump_test/dump_test.control
@@ -0,0 +1,5 @@
+# dump_test extension
+comment = 'dump_test - test pg_dump with extensions'
+default_version = '1.0'
+module_pathname = '$libdir/dump_test'
+relocatable = true
diff --git a/src/test/modules/dump_test/t/001_dump_test.pl b/src/test/modules/dump_test/t/001_dump_test.pl
new file mode 100644
index 0000000..18a1135
--- /dev/null
+++ b/src/test/modules/dump_test/t/001_dump_test.pl
@@ -0,0 +1,27 @@
+use strict;
+use warnings;
+use TestLib;
+use Test::More tests => 3;
+
+my $tempdir = tempdir;
+
+start_test_server $tempdir;
+
+psql 'postgres', 'CREATE EXTENSION dump_test';
+
+# Insert some data before running the dump, this is needed to check
+# consistent data dump of tables with foreign key dependencies
+psql 'postgres', 'INSERT INTO cc_tab_fkey VALUES (1)';
+psql 'postgres', 'INSERT INTO bb_tab_fkey VALUES (1)';
+psql 'postgres', 'INSERT INTO aa_tab_fkey VALUES (1)';
+
+# Create a table depending on a FK defined in the extension
+psql 'postgres', 'CREATE TABLE dd_tab_fkey (id int REFERENCES bb_tab_fkey(id))';
+psql 'postgres', 'INSERT INTO dd_tab_fkey VALUES (1)';
+
+# Take a dump then re-deploy it to a new database
+command_ok(['pg_dump', '-d', 'postgres', '-f', "$tempdir/dump.sql"],
+ 'taken dump with installed extension');
+command_ok(['createdb', 'foobar1'], 'created new database to redeploy dump');
+command_ok(['psql', '--set=ON_ERROR_STOP=on', '-d', 'foobar1', '-f',
+ "$tempdir/dump.sql"], 'dump restored');
--
2.3.0
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
