On Tue, Apr 13, 2021 at 08:00:34AM -0700, Noah Misch wrote: > On Tue, Apr 13, 2021 at 02:43:11PM +0900, Michael Paquier wrote: >>> - If extschema='public', "pg_dump -e plpgsql --schema=public" includes >>> commands to dump the relation data. This surprised me. (The >>> --schema=public argument causes selectDumpableNamespace() to set >>> nsinfo->dobj.dump=DUMP_COMPONENT_ALL instead of DUMP_COMPONENT_ACL.) >> >> This one would be expected to me. Per the discussion of upthread, we >> want --schema and --extension to be two separate and exclusive >> switches. So, once the caller specifies --schema we should dump the >> contents of the schema, even if its extension is not listed with >> --extension. > > I may disagree with this later, but I'm setting it aside for the moment. > > This isn't a problem of selecting schemas for inclusion in the dump. This is > a problem of associating extensions with their pg_extension_config_dump() > relations and omitting those extension-member relations when "-e" causes > omission of the extension.
At code level, the decision to dump the data of any extension's dumpable table is done in processExtensionTables(). I have to admit that your feeling here keeps the code simpler than what I have been thinking if we apply an extra filtering based on the list of extensions in this code path. So I can see the value in your argument to not dump at all the data of an extension's dumpable table as long as its extension is not listed, and this, even if its schema is explicitly listed. So I got down to make the behavior more consistent with the patch attached. This passes your case. It is worth noting that if a table is part of a schema created by an extension, but that the table is not dependent on the extension, we would still dump its data if using --schema with this table's schema while the extension is not part of the list from --extension. In the attached, that's just the extra test with without_extension_implicit_schema. (By the way, good catch with the duplicated --no-sync in the new tests.) What do you think? -- Michael
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index d0ea489614..391947340f 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -18271,7 +18271,8 @@ processExtensionTables(Archive *fout, ExtensionInfo extinfo[],
* Note that we create TableDataInfo objects even in schemaOnly mode, ie,
* user data in a configuration table is treated like schema data. This
* seems appropriate since system data in a config table would get
- * reloaded by CREATE EXTENSION.
+ * reloaded by CREATE EXTENSION. If the extension is not listed in the
+ * list of extensions to be included, none of its data is dumped.
*/
for (i = 0; i < numExtensions; i++)
{
@@ -18283,6 +18284,15 @@ processExtensionTables(Archive *fout, ExtensionInfo extinfo[],
int nconfigitems = 0;
int nconditionitems = 0;
+ /*
+ * Check if this extension is listed as to include in the dump. If
+ * not, any table data associated with it is discarded.
+ */
+ if (extension_include_oids.head != NULL &&
+ !simple_oid_list_member(&extension_include_oids,
+ curext->dobj.catId.oid))
+ continue;
+
if (strlen(extconfig) != 0 || strlen(extcondition) != 0)
{
int j;
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 ef98c08493..51450f1b3a 100644
--- a/src/test/modules/test_pg_dump/t/001_base.pl
+++ b/src/test/modules/test_pg_dump/t/001_base.pl
@@ -208,6 +208,30 @@ my %pgdump_runs = (
'pg_dump', '--no-sync', "--file=$tempdir/without_extension.sql",
'--extension=plpgsql', 'postgres',
],
+ },
+
+ # plgsql in the list of extensions blocks the dump of extension
+ # test_pg_dump.
+ without_extension_explicit_schema => {
+ dump_cmd => [
+ 'pg_dump',
+ '--no-sync',
+ "--file=$tempdir/without_extension_explicit_schema.sql",
+ '--extension=plpgsql',
+ '--schema=public',
+ 'postgres',
+ ],
+ },
+
+ without_extension_implicit_schema => {
+ dump_cmd => [
+ 'pg_dump',
+ '--no-sync',
+ "--file=$tempdir/without_extension_implicit_schema.sql",
+ '--extension=plpgsql',
+ '--schema=regress_pg_dump_schema',
+ 'postgres',
+ ],
},);
###############################################################
@@ -632,6 +656,8 @@ my %tests = (
pg_dumpall_globals => 1,
section_data => 1,
section_pre_data => 1,
+ # Excludes this schema as extension is not listed.
+ without_extension_explicit_schema => 1,
},
},
@@ -646,6 +672,8 @@ my %tests = (
pg_dumpall_globals => 1,
section_data => 1,
section_pre_data => 1,
+ # Excludes this schema as extension is not listed.
+ without_extension_explicit_schema => 1,
},
},
@@ -662,6 +690,8 @@ my %tests = (
%full_runs,
schema_only => 1,
section_pre_data => 1,
+ # Excludes the extension and keeps the schema's data.
+ without_extension_implicit_schema => 1,
},
},);
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 529b167c96..67c2cbbec6 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -234,6 +234,12 @@ PostgreSQL documentation
shell from expanding the wildcards.
</para>
+ <para>
+ Any configuration relation registered by
+ <function>pg_extension_config_dump</function> is included in the
+ dump if its extension is specified by <option>--extension</option>.
+ </para>
+
<note>
<para>
When <option>-e</option> is specified,
signature.asc
Description: PGP signature
