Allow pg_dump to dump non-extension members of an extension-owned schema.

tglsfdc · tglsfdc · commit df5d9bb8d507 · 2016-09-08T13:12:01.000-04:00
Previously, if a schema was created by an extension, a normal pg_dump run
(not --binary-upgrade) would summarily skip every object in that schema.
In a case where an extension creates a schema and then users create other
objects within that schema, this does the wrong thing: we want pg_dump
to skip the schema but still create the non-extension-owned objects.

There's no easy way to fix this pre-9.6, because in earlier versions the
"dump" status for a schema is just a bool and there's no way to distinguish
"dump me" from "dump my members".  However, as of 9.6 we do have enough
state to represent that, so this is a simple correction of the logic in
selectDumpableNamespace.

In passing, make some cosmetic fixes in nearby code.

Martín Marqués, reviewed by Michael Paquier

Discussion: &lt;99581032-71de-6466-c325-069861f1947d@2ndquadrant.com&gt;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
@@ -1302,7 +1302,7 @@ checkExtensionMembership(DumpableObject *dobj, Archive *fout)
 
 	/*
 	 * In 9.6 and above, mark the member object to have any non-initial ACL,
-	 * policies, and security lables dumped.
+	 * policies, and security labels dumped.
 	 *
 	 * Note that any initial ACLs (see pg_init_privs) will be removed when we
 	 * extract the information about the object.  We don't provide support for
@@ -1324,8 +1324,8 @@ checkExtensionMembership(DumpableObject *dobj, Archive *fout)
 			dobj->dump = DUMP_COMPONENT_NONE;
 		else
 			dobj->dump = ext->dobj.dump_contains & (DUMP_COMPONENT_ACL |
-							DUMP_COMPONENT_SECLABEL | DUMP_COMPONENT_POLICY);
-
+													DUMP_COMPONENT_SECLABEL |
+													DUMP_COMPONENT_POLICY);
 	}
 
 	return true;
@@ -1338,15 +1338,11 @@ 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
 	 * namespaces. Otherwise, dump all non-system namespaces.
 	 */
-
 	if (table_include_oids.head != NULL)
 		nsinfo->dobj.dump_contains = nsinfo->dobj.dump = DUMP_COMPONENT_NONE;
 	else if (schema_include_oids.head != NULL)
@@ -1355,18 +1351,21 @@ selectDumpableNamespace(NamespaceInfo *nsinfo, Archive *fout)
 								   nsinfo->dobj.catId.oid) ?
 			DUMP_COMPONENT_ALL : DUMP_COMPONENT_NONE;
 	else if (fout->remoteVersion >= 90600 &&
-			 strncmp(nsinfo->dobj.name, "pg_catalog",
-					 strlen("pg_catalog")) == 0)
-
+			 strcmp(nsinfo->dobj.name, "pg_catalog") == 0)
+	{
 		/*
 		 * In 9.6 and above, we dump out any ACLs defined in pg_catalog, if
 		 * they are interesting (and not the original ACLs which were set at
 		 * initdb time, see pg_init_privs).
 		 */
 		nsinfo->dobj.dump_contains = nsinfo->dobj.dump = DUMP_COMPONENT_ACL;
+	}
 	else if (strncmp(nsinfo->dobj.name, "pg_", 3) == 0 ||
 			 strcmp(nsinfo->dobj.name, "information_schema") == 0)
+	{
+		/* Other system schemas don't get dumped */
 		nsinfo->dobj.dump_contains = nsinfo->dobj.dump = DUMP_COMPONENT_NONE;
+	}
 	else
 		nsinfo->dobj.dump_contains = nsinfo->dobj.dump = DUMP_COMPONENT_ALL;
 
@@ -1377,6 +1376,15 @@ 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;
+
+	/*
+	 * If the schema belongs to an extension, allow extension membership to
+	 * override the dump decision for the schema itself.  However, this does
+	 * not change dump_contains, so this won't change what we do with objects
+	 * within the schema.  (If they belong to the extension, they'll get
+	 * suppressed by it, otherwise not.)
+	 */
+	(void) checkExtensionMembership(&nsinfo->dobj, fout);
 }
 
 /*
diff --git a/src/test/modules/test_pg_dump/t/001_base.pl b/src/test/modules/test_pg_dump/t/001_base.pl
@@ -429,7 +429,211 @@
 		unlike => {
 			no_privs           => 1,
 			pg_dumpall_globals => 1,
-			section_post_data  => 1, }, },);
+			section_post_data  => 1, }, },
+	# Objects included in extension part of a schema created by this extension */
+	'CREATE TABLE regress_pg_dump_schema.test_table' => {
+		regexp => qr/^
+			\QCREATE TABLE test_table (\E
+			\n\s+\Qcol1 integer,\E
+			\n\s+\Qcol2 integer\E
+			\n\);$/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, }, },
+	'GRANT SELECT ON regress_pg_dump_schema.test_table' => {
+		regexp => qr/^
+			\QSELECT pg_catalog.binary_upgrade_set_record_init_privs(true);\E\n
+			\QGRANT SELECT ON TABLE test_table TO regress_dump_test_role;\E\n
+			\QSELECT pg_catalog.binary_upgrade_set_record_init_privs(false);\E
+			$/xms,
+		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, }, },
+	'CREATE SEQUENCE regress_pg_dump_schema.test_seq' => {
+		regexp => qr/^
+                    \QCREATE SEQUENCE test_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, }, },
+	'GRANT USAGE ON regress_pg_dump_schema.test_seq' => {
+		regexp => qr/^
+			\QSELECT pg_catalog.binary_upgrade_set_record_init_privs(true);\E\n
+			\QGRANT USAGE ON SEQUENCE test_seq TO regress_dump_test_role;\E\n
+			\QSELECT pg_catalog.binary_upgrade_set_record_init_privs(false);\E
+			$/xms,
+		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, }, },
+	'CREATE TYPE regress_pg_dump_schema.test_type' => {
+		regexp => qr/^
+                    \QCREATE TYPE test_type AS (\E
+                    \n\s+\Qcol1 integer\E
+                    \n\);$/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, }, },
+	'GRANT USAGE ON regress_pg_dump_schema.test_type' => {
+		regexp => qr/^
+			\QSELECT pg_catalog.binary_upgrade_set_record_init_privs(true);\E\n
+			\QGRANT ALL ON TYPE test_type TO regress_dump_test_role;\E\n
+			\QSELECT pg_catalog.binary_upgrade_set_record_init_privs(false);\E
+			$/xms,
+		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, }, },
+	'CREATE FUNCTION regress_pg_dump_schema.test_func' => {
+		regexp => qr/^
+            \QCREATE FUNCTION test_func() RETURNS integer\E
+            \n\s+\QLANGUAGE sql\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, }, },
+	'GRANT ALL ON regress_pg_dump_schema.test_func' => {
+		regexp => qr/^
+			\QSELECT pg_catalog.binary_upgrade_set_record_init_privs(true);\E\n
+			\QGRANT ALL ON FUNCTION test_func() TO regress_dump_test_role;\E\n
+			\QSELECT pg_catalog.binary_upgrade_set_record_init_privs(false);\E
+			$/xms,
+		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, }, },
+	'CREATE AGGREGATE regress_pg_dump_schema.test_agg' => {
+		regexp => qr/^
+            \QCREATE AGGREGATE test_agg(smallint) (\E
+            \n\s+\QSFUNC = int2_sum,\E
+            \n\s+\QSTYPE = bigint\E
+            \n\);$/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, }, },
+	'GRANT ALL ON regress_pg_dump_schema.test_agg' => {
+		regexp => qr/^
+			\QSELECT pg_catalog.binary_upgrade_set_record_init_privs(true);\E\n
+			\QGRANT ALL ON FUNCTION test_agg(smallint) TO regress_dump_test_role;\E\n
+			\QSELECT pg_catalog.binary_upgrade_set_record_init_privs(false);\E
+			$/xms,
+		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, }, },
+	# Objects not included in extension, part of schema created by extension
+	'CREATE TABLE regress_pg_dump_schema.external_tab' => {
+		create_order => 4,
+		create_sql   => 'CREATE TABLE regress_pg_dump_schema.external_tab
+						   (col1 int);',
+		regexp => qr/^
+			\QCREATE TABLE external_tab (\E
+			\n\s+\Qcol1 integer\E
+			\n\);$/xm,
+		like => {
+			binary_upgrade   => 1,
+			clean            => 1,
+			clean_if_exists  => 1,
+			createdb         => 1,
+			defaults         => 1,
+			no_owner         => 1,
+			no_privs           => 1,
+			schema_only      => 1,
+			section_pre_data => 1, },
+		unlike => {
+			pg_dumpall_globals => 1,
+			section_post_data  => 1, }, }, );
 
 #########################################
 # Create a PG instance to test actually dumping from
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
@@ -10,6 +10,8 @@ CREATE TABLE regress_pg_dump_table (
 
 CREATE SEQUENCE regress_pg_dump_seq;
 
+CREATE SCHEMA regress_pg_dump_schema;
+
 GRANT USAGE ON regress_pg_dump_seq TO regress_dump_test_role;
 
 GRANT SELECT ON regress_pg_dump_table TO regress_dump_test_role;
@@ -19,3 +21,25 @@ GRANT SELECT(col2) ON regress_pg_dump_table TO regress_dump_test_role;
 REVOKE SELECT(col2) ON regress_pg_dump_table FROM regress_dump_test_role;
 
 CREATE ACCESS METHOD regress_test_am TYPE INDEX HANDLER bthandler;
+
+-- Create a set of objects that are part of the schema created by
+-- this extension.
+CREATE TABLE regress_pg_dump_schema.test_table (
+	col1 int,
+	col2 int
+);
+GRANT SELECT ON regress_pg_dump_schema.test_table TO regress_dump_test_role;
+
+CREATE SEQUENCE regress_pg_dump_schema.test_seq;
+GRANT USAGE ON regress_pg_dump_schema.test_seq TO regress_dump_test_role;
+
+CREATE TYPE regress_pg_dump_schema.test_type AS (col1 int);
+GRANT USAGE ON TYPE regress_pg_dump_schema.test_type TO regress_dump_test_role;
+
+CREATE FUNCTION regress_pg_dump_schema.test_func () RETURNS int
+AS 'SELECT 1;' LANGUAGE SQL;
+GRANT EXECUTE ON FUNCTION regress_pg_dump_schema.test_func() TO regress_dump_test_role;
+
+CREATE AGGREGATE regress_pg_dump_schema.test_agg(int2)
+(SFUNC = int2_sum, STYPE = int8);
+GRANT EXECUTE ON FUNCTION regress_pg_dump_schema.test_agg(int2) TO regress_dump_test_role;
