From f9e439b1ca81e3305b677d93c67299549625370c Mon Sep 17 00:00:00 2001 From: Stephen Frost Date: Sun, 31 Jul 2016 10:57:15 -0400 Subject: [PATCH] Correctly handle owned sequences with extensions With the refactoring of pg_dump to handle components, getOwnedSeqs needs to be a bit more intelligent regarding which components to dump when. Specifically, we can't simply use the owning table's components as the set of components to dump as the table might only be including certain components while all components of the sequence should be dumped, for example, when the table is a member of an extension while the sequence is not. Handle this by combining the set of components to be dumped for the sequence explicitly and those to be dumped for the table when setting the components to be dumped for the sequence. Also add a number of regression tests around this to, hopefully, catch any future changes which break the expected behavior. Discovered by: Philippe BEAUDOIN Reviewed by: Michael Paquier --- src/bin/pg_dump/pg_dump.c | 27 +++++-- src/bin/pg_dump/t/002_pg_dump.pl | 50 ++++++++++++ src/test/modules/test_pg_dump/t/001_base.pl | 81 ++++++++++++++++++- .../test_pg_dump/test_pg_dump--1.0.sql | 6 +- 4 files changed, 155 insertions(+), 9 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 08c2b0c7a3..4ee10fcb39 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -6035,14 +6035,27 @@ getOwnedSeqs(Archive *fout, TableInfo tblinfo[], int numTables) if (!OidIsValid(seqinfo->owning_tab)) continue; /* not an owned sequence */ - if (seqinfo->dobj.dump & DUMP_COMPONENT_DEFINITION) - continue; /* no need to search */ + owning_tab = findTableByOid(seqinfo->owning_tab); - if (owning_tab && owning_tab->dobj.dump) - { + + /* + * We need to dump the components that are being dumped for the table + * and any components which the sequence is explicitly marked with. + * + * We can't simply use the set of components which are being dumped for + * the table as the table might be in an extension (and only the + * non-extension components, eg: ACLs if changed, security labels, and + * policies, are being dumped) while the sequence is not (and therefore + * the definition and other components should also be dumped). + * + * If the sequence is part of the extension then it should be properly + * marked by checkExtensionMembership() and this will be a no-op as the + * table will be equivalently marked. + */ + seqinfo->dobj.dump = seqinfo->dobj.dump | owning_tab->dobj.dump; + + if (seqinfo->dobj.dump != DUMP_COMPONENT_NONE) seqinfo->interesting = true; - seqinfo->dobj.dump = DUMP_COMPONENT_ALL; - } } } @@ -16465,7 +16478,7 @@ dumpSequence(Archive *fout, TableInfo *tbinfo) { TableInfo *owning_tab = findTableByOid(tbinfo->owning_tab); - if (owning_tab && owning_tab->dobj.dump) + if (owning_tab && owning_tab->dobj.dump & DUMP_COMPONENT_DEFINITION) { resetPQExpBuffer(query); appendPQExpBuffer(query, "ALTER SEQUENCE %s", diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index 1d82bfd047..5a042b8552 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -361,6 +361,56 @@ my %tests = ( only_dump_test_schema => 1, only_dump_test_table => 1, test_schema_plus_blobs => 1, }, }, + 'ALTER SEQUENCE test_table_col1_seq' => { + regexp => qr/^ + \QALTER SEQUENCE test_table_col1_seq OWNED BY test_table.col1;\E + /xm, + like => { + binary_upgrade => 1, + clean => 1, + clean_if_exists => 1, + createdb => 1, + defaults => 1, + exclude_test_table_data => 1, + no_privs => 1, + no_owner => 1, + only_dump_test_schema => 1, + only_dump_test_table => 1, + pg_dumpall_dbprivs => 1, + schema_only => 1, + section_pre_data => 1, + test_schema_plus_blobs => 1, }, + unlike => { + exclude_test_table => 1, + exclude_dump_test_schema => 1, + pg_dumpall_globals => 1, + pg_dumpall_globals_clean => 1, + section_post_data => 1, }, }, + 'ALTER SEQUENCE test_third_table_col1_seq' => { + regexp => qr/^ + \QALTER SEQUENCE test_third_table_col1_seq OWNED BY test_third_table.col1;\E + /xm, + like => { + binary_upgrade => 1, + clean => 1, + clean_if_exists => 1, + createdb => 1, + defaults => 1, + exclude_dump_test_schema => 1, + exclude_test_table => 1, + exclude_test_table_data => 1, + no_privs => 1, + no_owner => 1, + pg_dumpall_dbprivs => 1, + schema_only => 1, + section_pre_data => 1, }, + unlike => { + only_dump_test_schema => 1, + only_dump_test_table => 1, + pg_dumpall_globals => 1, + pg_dumpall_globals_clean => 1, + section_post_data => 1, + test_schema_plus_blobs => 1, }, }, 'ALTER TABLE ONLY test_table ADD CONSTRAINT ... PRIMARY KEY' => { regexp => qr/^ \QALTER TABLE ONLY test_table\E \n^\s+ 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 fd9c37faaf..fb4f5737e5 100644 --- a/src/test/modules/test_pg_dump/t/001_base.pl +++ b/src/test/modules/test_pg_dump/t/001_base.pl @@ -223,10 +223,52 @@ my %tests = ( schema_only => 1, section_pre_data => 1, section_post_data => 1, }, }, + 'CREATE SEQUENCE regress_pg_dump_table_col1_seq' => { + regexp => qr/^ + \QCREATE SEQUENCE regress_pg_dump_table_col1_seq\E + \n\s+\QSTART WITH 1\E + \n\s+\QINCREMENT BY 1\E + \n\s+\QNO MINVALUE\E + \n\s+\QNO MAXVALUE\E + \n\s+\QCACHE 1;\E + $/xm, + like => { binary_upgrade => 1, }, + unlike => { + clean => 1, + clean_if_exists => 1, + createdb => 1, + defaults => 1, + no_privs => 1, + no_owner => 1, + pg_dumpall_globals => 1, + schema_only => 1, + section_pre_data => 1, + section_post_data => 1, }, }, + 'CREATE SEQUENCE regress_pg_dump_seq' => { + regexp => qr/^ + \QCREATE SEQUENCE regress_pg_dump_seq\E + \n\s+\QSTART WITH 1\E + \n\s+\QINCREMENT BY 1\E + \n\s+\QNO MINVALUE\E + \n\s+\QNO MAXVALUE\E + \n\s+\QCACHE 1;\E + $/xm, + like => { binary_upgrade => 1, }, + unlike => { + clean => 1, + clean_if_exists => 1, + createdb => 1, + defaults => 1, + no_privs => 1, + no_owner => 1, + pg_dumpall_globals => 1, + schema_only => 1, + section_pre_data => 1, + section_post_data => 1, }, }, 'CREATE TABLE regress_pg_dump_table' => { regexp => qr/^ \QCREATE TABLE regress_pg_dump_table (\E - \n\s+\Qcol1 integer,\E + \n\s+\Qcol1 integer NOT NULL,\E \n\s+\Qcol2 integer\E \n\);$/xm, like => { binary_upgrade => 1, }, @@ -331,6 +373,43 @@ my %tests = ( no_privs => 1, pg_dumpall_globals => 1, section_post_data => 1, }, }, + 'GRANT USAGE ON regress_pg_dump_table_col1_seq TO regress_dump_test_role' => { + create_order => 5, + create_sql => 'GRANT USAGE ON SEQUENCE regress_pg_dump_table_col1_seq + TO regress_dump_test_role;', + regexp => qr/^ + \QGRANT USAGE ON SEQUENCE regress_pg_dump_table_col1_seq TO regress_dump_test_role;\E + $/xm, + like => { + binary_upgrade => 1, + clean => 1, + clean_if_exists => 1, + createdb => 1, + defaults => 1, + no_owner => 1, + schema_only => 1, + section_pre_data => 1, }, + unlike => { + no_privs => 1, + pg_dumpall_globals => 1, + section_post_data => 1, }, }, + 'GRANT USAGE ON regress_pg_dump_seq TO regress_dump_test_role' => { + regexp => qr/^ + \QGRANT USAGE ON SEQUENCE regress_pg_dump_seq TO regress_dump_test_role;\E + $/xm, + like => { + binary_upgrade => 1, }, + unlike => { + clean => 1, + clean_if_exists => 1, + createdb => 1, + defaults => 1, + no_owner => 1, + no_privs => 1, + pg_dumpall_globals => 1, + schema_only => 1, + section_pre_data => 1, + section_post_data => 1, }, }, 'REVOKE SELECT(col1) ON regress_pg_dump_table' => { create_order => 3, create_sql => 'REVOKE SELECT(col1) ON regress_pg_dump_table 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 5fe606365e..c2fe90d5ab 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 @@ -4,10 +4,14 @@ \echo Use "CREATE EXTENSION test_pg_dump" to load this file. \quit CREATE TABLE regress_pg_dump_table ( - col1 int, + col1 serial, col2 int ); +CREATE SEQUENCE regress_pg_dump_seq; + +GRANT USAGE ON regress_pg_dump_seq TO regress_dump_test_role; + GRANT SELECT ON regress_pg_dump_table TO regress_dump_test_role; GRANT SELECT(col1) ON regress_pg_dump_table TO public;