On Wed, Jul 27, 2016 at 8:07 AM, Stephen Frost <sfr...@snowman.net> wrote:
> That'd be great.  It's definitely on my list of things to look into, but
> I'm extremely busy this week.  I hope to look into it on Friday, would
> be great to see what you find.

Sequences that are directly defined in extensions do not get dumped,
and sequences that are part of a serial column in an extension are
getting dumped. Looking into this problem, getOwnedSeqs() is visibly
doing an incorrect assumption: sequences owned by table columns are
dumped unconditionally, but this is not true for sequences that are
part of extensions. More precisely, dobj->dump is being enforced to
DUMP_COMPONENT_ALL, which makes the sequence definition to be dumped.
Oops.

The patch attached fixes the problem for me. I have added as well
tests in test_pg_dump in the shape of sequences defined in an
extension, and sequences that are part of a serial column. This patch
is also able to work in the case where a sequence is created as part
of a serial column, and gets removed after, say with ALTER EXTENSION
DROP SEQUENCE. The behavior for sequences and serial columns that are
not part of extensions is unchanged.

Stephen, it would be good if you could check the correctness of this
patch as you did all this refactoring of pg_dump to support catalog
ACLs. I am sure by the way that checking for (owning_tab->dobj.dump &&
DUMP_COMPONENT_DEFINITION) != 0 is not good because of for example the
case of a serial column created in an extension where the sequence is
dropped from the extension afterwards.
-- 
Michael
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 08c2b0c..0278995 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -6037,6 +6037,8 @@ getOwnedSeqs(Archive *fout, TableInfo tblinfo[], int numTables)
 			continue;			/* not an owned sequence */
 		if (seqinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 			continue;			/* no need to search */
+		if (seqinfo->dobj.ext_member)
+			continue;			/* member of an extension */
 		owning_tab = findTableByOid(seqinfo->owning_tab);
 		if (owning_tab && owning_tab->dobj.dump)
 		{
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 fd9c37f..9caee93 100644
--- a/src/test/modules/test_pg_dump/t/001_base.pl
+++ b/src/test/modules/test_pg_dump/t/001_base.pl
@@ -226,7 +226,7 @@ my %tests = (
 	'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, },
@@ -241,6 +241,48 @@ 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 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 5fe6063..93de2c5 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,12 @@
 \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 SELECT ON regress_pg_dump_table TO regress_dump_test_role;
 GRANT SELECT(col1) ON regress_pg_dump_table TO public;
 
-- 
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general

Reply via email to