Hi Michael,
2016-08-23 5:02 GMT-03:00 Michael Paquier <michael.paqu...@gmail.com>:
> On Sat, Aug 13, 2016 at 6:58 AM, Martín Marqués <mar...@2ndquadrant.com>
> wrote:
>> I believe the fix will be simple after the back and forth mails with
>> Michael, Stephen and Tom. I will work on that later, but preferred to
>> have the tests the show the problem which will also make testing the fix
>> easier.
>>
>> Thoughts?
>
> It seems to me that we are going to need a bit more coverage for more
> object types depending on the code paths that are being changed. For
> example, sequences or functions should be checked for as well, and not
> only tables. By the way, do you need help to build a patch or should I
> jump in?
I wanted to test what I had in mind with one object, and then see if
any replication is needed for other objects.
I was struggling the last days as what I was reading in my patched
pg_dump.c had to work as expected, and dump the tables not created by
the test_pg_dump extension but inside the schema
regress_pg_dump_schema.
Today I decided to go over the test I wrote, and found a bug there,
reason why I couldn't get a successful make check.
Here go 2 patches. One is a fix for the test I sent earlier. The other
is the proposed idea Tom had using the dump_contains that Stephan
committed on 9.6.
So far I've checked that it fixes the dumpable for Tables, but I think
it should work for all other objects as well, as all this patch does
is leave execution of checkExtensionMembership at the end of
selectDumpableNamespace, leaving the dump_contains untouched.
Checks pass ok.
I will add tests for sequence and functions as you mention and test again.
Then I'll check if other tests should be added as well.
--
Martín Marqués http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From 6cbba9e87d1733ccb03d3d705e135e607be63cba Mon Sep 17 00:00:00 2001
From: Martin <mar...@2ndquadrant.com>
Date: Tue, 23 Aug 2016 16:17:38 -0300
Subject: [PATCH 1/2] New tests for pg_dump which make sure that tables from a
schema depending on an extension will get dumped when they don't depend on
the extension.
The only objects we shouldn't dump are the ones created by the extension.
We will provide a patch that fixes this bug.
---
src/test/modules/test_pg_dump/t/001_base.pl | 25 ++++++++++++++++++++++
.../modules/test_pg_dump/test_pg_dump--1.0.sql | 9 ++++++++
2 files changed, 34 insertions(+)
diff --git a/src/test/modules/test_pg_dump/t/001_base.pl b/src/test/modules/test_pg_dump/t/001_base.pl
index fb4f573..7bb92e7 100644
--- a/src/test/modules/test_pg_dump/t/001_base.pl
+++ b/src/test/modules/test_pg_dump/t/001_base.pl
@@ -283,6 +283,31 @@ my %tests = (
schema_only => 1,
section_pre_data => 1,
section_post_data => 1, }, },
+ 'CREATE TABLE regress_test_schema_table' => {
+ create_order => 3,
+ create_sql => 'CREATE TABLE regress_pg_dump_schema.test_schema_table (
+ col1 serial primary key,
+ CHECK (col1 <= 1000)
+ );',
+ regexp => qr/^
+ \QCREATE TABLE test_schema_table (\E
+ \n\s+\Qcol1 integer NOT NULL,\E
+ \n\s+\QCONSTRAINT test_schema_table_col1_check CHECK \E
+ \Q((col1 <= 1000))\E
+ \n\);/xm,
+ like => {
+ binary_upgrade => 1,
+ clean => 1,
+ clean_if_exists => 1,
+ createdb => 1,
+ defaults => 1,
+ no_privs => 1,
+ no_owner => 1,
+ schema_only => 1,
+ section_pre_data => 1, },
+ unlike => {
+ pg_dumpall_globals => 1,
+ section_post_data => 1, }, },
'CREATE ACCESS METHOD regress_test_am' => {
regexp => qr/^
\QCREATE ACCESS METHOD regress_test_am TYPE INDEX HANDLER bthandler;\E
diff --git a/src/test/modules/test_pg_dump/test_pg_dump--1.0.sql b/src/test/modules/test_pg_dump/test_pg_dump--1.0.sql
index c2fe90d..3f88e6c 100644
--- a/src/test/modules/test_pg_dump/test_pg_dump--1.0.sql
+++ b/src/test/modules/test_pg_dump/test_pg_dump--1.0.sql
@@ -10,6 +10,15 @@ CREATE TABLE regress_pg_dump_table (
CREATE SEQUENCE regress_pg_dump_seq;
+-- We want to test that schemas and objects created in the schema by the
+-- extension are not dumped, yet other objects created afterwards will be
+-- dumped.
+CREATE SCHEMA regress_pg_dump_schema
+ CREATE TABLE regress_pg_dump_schema_table (
+ col1 serial,
+ col2 int
+ );
+
GRANT USAGE ON regress_pg_dump_seq TO regress_dump_test_role;
GRANT SELECT ON regress_pg_dump_table TO regress_dump_test_role;
--
2.5.5
From 0dfe2224fae86ab2b6f8da4fc25b96fb13ca0f6c Mon Sep 17 00:00:00 2001
From: Martin <mar...@2ndquadrant.com>
Date: Tue, 23 Aug 2016 16:23:44 -0300
Subject: [PATCH 2/2] This patch fixes a bug reported against pg_dump, and
makes pg_dump dump the objects contained in schemas depending on an
extension.
Schema will not be dumped, as it will be created by the extension, but
any other tables created outside the extension's SQL will be dumped.
We need to provide further patches for other objects like sequences,
functions, etc.
---
src/bin/pg_dump/pg_dump.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 4ee10fc..13b61bd 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1338,9 +1338,6 @@ checkExtensionMembership(DumpableObject *dobj, Archive *fout)
static void
selectDumpableNamespace(NamespaceInfo *nsinfo, Archive *fout)
{
- if (checkExtensionMembership(&nsinfo->dobj, fout))
- return; /* extension membership overrides all else */
-
/*
* If specific tables are being dumped, do not dump any complete
* namespaces. If specific namespaces are being dumped, dump just those
@@ -1377,6 +1374,13 @@ selectDumpableNamespace(NamespaceInfo *nsinfo, Archive *fout)
simple_oid_list_member(&schema_exclude_oids,
nsinfo->dobj.catId.oid))
nsinfo->dobj.dump_contains = nsinfo->dobj.dump = DUMP_COMPONENT_NONE;
+
+ /*
+ * Extension membership overrides all else, with the exception of objects
+ * dump_contains.
+ */
+ checkExtensionMembership(&nsinfo->dobj, fout);
+
}
/*
--
2.5.5
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
