Extensions not dumped when --schema is used
Hello,
I've discovered something today that I didn't really expect. When a user
dumps a database with the --schema flag of pg_dump, extensions in this
schema aren't dumped. As far as I can tell, the documentation isn't clear
about this ("Dump only schemas matching pattern; this selects both the
schema itself, and all its contained objects."), though the source code
definitely is ("We dump all user-added extensions by default, or none of
them if include_everything is false (i.e., a --schema or --table switch was
given).", in pg_dump.c).
I was wondering the reason behind this choice. If anyone knows, I'd be
happy to hear about it.
I see two things:
* it's been overlooked, and we should dump all the extensions available in
a schema if this schema has been selected through the --schema flag.
* it's kind of like the large objects handling, and I'd pretty interested
in adding a --extensions (as the same way there is a --blobs flag).
Thanks.
Regards.
--
Guillaume.
On Wed, 2020-05-20 at 10:06 +0200, Guillaume Lelarge wrote:
I've discovered something today that I didn't really expect.
When a user dumps a database with the --schema flag of pg_dump,
extensions in this schema aren't dumped. As far as I can tell,
the documentation isn't clear about this ("Dump only schemas
matching pattern; this selects both the schema itself, and all
its contained objects."), though the source code definitely is
("We dump all user-added extensions by default, or none of them
if include_everything is false (i.e., a --schema or --table
switch was given).", in pg_dump.c).I was wondering the reason behind this choice. If anyone knows,
I'd be happy to hear about it.I see two things:
* it's been overlooked, and we should dump all the extensions
available in a schema if this schema has been selected through
the --schema flag.
* it's kind of like the large objects handling, and I'd pretty
interested in adding a --extensions (as the same way there is a
--blobs flag).
I am not sure if there is a good reason for the current behavior,
but I'd favor the second solution. I think as extensions as belonging
to the database rather than the schema; the schema is just where the
objects are housed.
Yours,
Laurenz Albe
On 20 May 2020, at 10:06, Guillaume Lelarge <guillaume@lelarge.info> wrote:
I was wondering the reason behind this choice. If anyone knows, I'd be happy to hear about it.
Extensions were dumped unconditionally in the beginning, but it was changed to
match how procedural language definitions were dumped.
* it's been overlooked, and we should dump all the extensions available in a schema if this schema has been selected through the --schema flag.
For reference, --schema-only will include all the extensions, but not
--schema=foo and not "--schema-only --schema=foo".
Extensions don't belong to a schema per se, the namespace oid in pg_extension
marks where most of the objects are contained but not necessarily all of them.
Given that, it makes sense to not include extensions for --schema. However,
that can be considered sort of an implementation detail which may not be
entirely obvious to users (especially since you yourself is a power-user).
* it's kind of like the large objects handling, and I'd pretty interested in adding a --extensions (as the same way there is a --blobs flag).
An object in a schema might have attributes that depend on an extension in
order to restore, so it makes sense to provide a way to include them for a
--schema dump. The question is what --extensions should do: only dump any
extensions that objects in the schema depend on; require a pattern and only
dump matching extensions; dump all extensions (probably not) or something else?
cheers ./daniel
Le mer. 20 mai 2020 à 11:26, Daniel Gustafsson <daniel@yesql.se> a écrit :
On 20 May 2020, at 10:06, Guillaume Lelarge <guillaume@lelarge.info>
wrote:
I was wondering the reason behind this choice. If anyone knows, I'd be
happy to hear about it.
Extensions were dumped unconditionally in the beginning, but it was
changed to
match how procedural language definitions were dumped.
That makes sense. Thank you.
* it's been overlooked, and we should dump all the extensions available
in a schema if this schema has been selected through the --schema flag.For reference, --schema-only will include all the extensions, but not
--schema=foo and not "--schema-only --schema=foo".
Yes.
Extensions don't belong to a schema per se, the namespace oid in
pg_extension
marks where most of the objects are contained but not necessarily all of
them.
Given that, it makes sense to not include extensions for --schema.
However,
that can be considered sort of an implementation detail which may not be
entirely obvious to users (especially since you yourself is a power-user).
I agree.
* it's kind of like the large objects handling, and I'd pretty interested
in adding a --extensions (as the same way there is a --blobs flag).An object in a schema might have attributes that depend on an extension in
order to restore, so it makes sense to provide a way to include them for a
--schema dump.
That's actually how I figured this out. A customer can't restore his dump
because of a missing extension (pg_trgm to be precise).
The question is what --extensions should do: only dump any
extensions that objects in the schema depend on; require a pattern and only
dump matching extensions; dump all extensions (probably not) or something
else?
Actually, "dump all extensions" (#3) would make sense to me, and has my
vote. Otherwise, and though it would imply much more work, "only dump any
extension that objects in the schema depend on" (#1) comes second in my
opinion. Using the pattern means something more manual for users, and I
really think it would be a bad idea. People dump databases, schemas, and
tables. Theu usually don't know which extensions those objects depend on.
But, anyway, I would work on any of these solutions, depending on what most
people think is best.
Thanks.
--
Guillaume.
On 20 May 2020, at 11:38, Guillaume Lelarge <guillaume@lelarge.info> wrote:
Actually, "dump all extensions" (#3) would make sense to me, and has my vote.
Wouldn't that open for another set of problems when running with --schema=bar
and getting errors on restoring for relocatable extensions like these:
CREATE EXTENSION IF NOT EXISTS pg_trgm WITH SCHEMA foo;
Only dumping extensions depended on has the same problem of course.
People dump databases, schemas, and tables. Theu usually don't know which extensions those objects depend on.
That I totally agree with, question is how we best can help users here.
cheers ./daniel
Guillaume Lelarge <guillaume@lelarge.info> writes:
Le mer. 20 mai 2020 à 11:26, Daniel Gustafsson <daniel@yesql.se> a écrit :
The question is what --extensions should do: only dump any
extensions that objects in the schema depend on; require a pattern and only
dump matching extensions; dump all extensions (probably not) or something
else?
Actually, "dump all extensions" (#3) would make sense to me, and has my
vote.
I think that makes no sense at all. By definition, a dump that's been
restricted with --schema, --table, or any similar switch is incomplete
and may not restore on its own. Typical examples include foreign key
references to tables in other schemas, views using functions in other
schemas, etc etc. I see no reason for extension dependencies to be
treated differently from those cases.
In any use of selective dump, it's the user's job to select a set of
objects that she wants dumped (or restored). Trying to second-guess that
is mostly going to make the feature less usable for power-user cases.
As a counterexample, what if you want the dump to be restorable on a
system that doesn't have all of the extensions available on the source?
You carefully pick out the tables that you need, which don't require the
unavailable extensions ... and then pg_dump decides you don't know what
you're doing and includes all the problematic extensions anyway.
I could get behind an "--extensions=PATTERN" switch to allow selective
addition of extensions to a selective dump, but I don't want to see us
overruling the user's choices about what to dump.
regards, tom lane
Le mer. 20 mai 2020 à 16:39, Tom Lane <tgl@sss.pgh.pa.us> a écrit :
Guillaume Lelarge <guillaume@lelarge.info> writes:
Le mer. 20 mai 2020 à 11:26, Daniel Gustafsson <daniel@yesql.se> a
écrit :
The question is what --extensions should do: only dump any
extensions that objects in the schema depend on; require a pattern andonly
dump matching extensions; dump all extensions (probably not) or
something
else?
Actually, "dump all extensions" (#3) would make sense to me, and has my
vote.I think that makes no sense at all. By definition, a dump that's been
restricted with --schema, --table, or any similar switch is incomplete
and may not restore on its own. Typical examples include foreign key
references to tables in other schemas, views using functions in other
schemas, etc etc. I see no reason for extension dependencies to be
treated differently from those cases.
Agreed.
In any use of selective dump, it's the user's job to select a set of
objects that she wants dumped (or restored). Trying to second-guess that
is mostly going to make the feature less usable for power-user cases.
Agreed, though right now he has no way to do this for extensions.
As a counterexample, what if you want the dump to be restorable on a
system that doesn't have all of the extensions available on the source?
You carefully pick out the tables that you need, which don't require the
unavailable extensions ... and then pg_dump decides you don't know what
you're doing and includes all the problematic extensions anyway.
That's true.
I could get behind an "--extensions=PATTERN" switch to allow selective
addition of extensions to a selective dump, but I don't want to see us
overruling the user's choices about what to dump.
With all your comments, I can only agree to your views. I'll try to work on
this anytime soon.
--
Guillaume.
Hi,
Le sam. 23 mai 2020 à 14:53, Guillaume Lelarge <guillaume@lelarge.info> a
écrit :
Le mer. 20 mai 2020 à 16:39, Tom Lane <tgl@sss.pgh.pa.us> a écrit :
Guillaume Lelarge <guillaume@lelarge.info> writes:
Le mer. 20 mai 2020 à 11:26, Daniel Gustafsson <daniel@yesql.se> a
écrit :
The question is what --extensions should do: only dump any
extensions that objects in the schema depend on; require a pattern andonly
dump matching extensions; dump all extensions (probably not) or
something
else?
Actually, "dump all extensions" (#3) would make sense to me, and has my
vote.I think that makes no sense at all. By definition, a dump that's been
restricted with --schema, --table, or any similar switch is incomplete
and may not restore on its own. Typical examples include foreign key
references to tables in other schemas, views using functions in other
schemas, etc etc. I see no reason for extension dependencies to be
treated differently from those cases.Agreed.
In any use of selective dump, it's the user's job to select a set of
objects that she wants dumped (or restored). Trying to second-guess that
is mostly going to make the feature less usable for power-user cases.Agreed, though right now he has no way to do this for extensions.
As a counterexample, what if you want the dump to be restorable on a
system that doesn't have all of the extensions available on the source?
You carefully pick out the tables that you need, which don't require the
unavailable extensions ... and then pg_dump decides you don't know what
you're doing and includes all the problematic extensions anyway.That's true.
I could get behind an "--extensions=PATTERN" switch to allow selective
addition of extensions to a selective dump, but I don't want to see us
overruling the user's choices about what to dump.With all your comments, I can only agree to your views. I'll try to work
on this anytime soon.
"Anytime soon" was a long long time ago, and I eventually completely forgot
this, sorry. As nobody worked on it yet, I took a shot at it. See attached
patch.
I don't know if I should add this right away in the commit fest app. If
yes, I guess it should go on the next commit fest (2021-03), right?
--
Guillaume.
Attachments:
dump_extensions_v1.patchtext/x-patch; charset=US-ASCII; name=dump_extensions_v1.patchDownload
commit 97c41b05b4aa20f4187aedbed79d67874d2cbbbd
Author: Guillaume Lelarge <guillaume@lelarge.info>
Date: Mon Jan 25 14:25:53 2021 +0100
Add a --extension flag to dump specific extensions
Version 1.
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index bcbb7a25fb..95d45fabfb 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -215,6 +215,38 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>-e <replaceable class="parameter">pattern</replaceable></option></term>
+ <term><option>--extension=<replaceable class="parameter">pattern</replaceable></option></term>
+ <listitem>
+ <para>
+ Dump only extensions matching <replaceable
+ class="parameter">pattern</replaceable>. When this option is not
+ specified, all non-system extensions in the target database will be
+ dumped. Multiple schemas can be selected by writing multiple
+ <option>-e</option> switches. The <replaceable
+ class="parameter">pattern</replaceable> parameter is interpreted as a
+ pattern according to the same rules used by
+ <application>psql</application>'s <literal>\d</literal> commands (see
+ <xref linkend="app-psql-patterns"/>), so multiple extensions can also
+ be selected by writing wildcard characters in the pattern. When using
+ wildcards, be careful to quote the pattern if needed to prevent the
+ shell from expanding the wildcards.
+ </para>
+
+ <note>
+ <para>
+ When <option>-e</option> is specified,
+ <application>pg_dump</application> makes no attempt to dump any other
+ database objects that the selected extension(s) might depend upon.
+ Therefore, there is no guarantee that the results of a
+ specific-extension dump can be successfully restored by themselves
+ into a clean database.
+ </para>
+ </note>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>-E <replaceable class="parameter">encoding</replaceable></option></term>
<term><option>--encoding=<replaceable class="parameter">encoding</replaceable></option></term>
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 798d14580e..3c1203e85c 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -123,6 +123,9 @@ static SimpleOidList tabledata_exclude_oids = {NULL, NULL};
static SimpleStringList foreign_servers_include_patterns = {NULL, NULL};
static SimpleOidList foreign_servers_include_oids = {NULL, NULL};
+static SimpleStringList extension_include_patterns = {NULL, NULL};
+static SimpleOidList extension_include_oids = {NULL, NULL};
+
static const CatalogId nilCatalogId = {0, 0};
/* override for standard extra_float_digits setting */
@@ -151,6 +154,10 @@ static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids,
bool strict_names);
+static void expand_extension_name_patterns(Archive *fout,
+ SimpleStringList *patterns,
+ SimpleOidList *oids,
+ bool strict_names);
static void expand_foreign_server_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids);
@@ -334,6 +341,7 @@ main(int argc, char **argv)
{"clean", no_argument, NULL, 'c'},
{"create", no_argument, NULL, 'C'},
{"dbname", required_argument, NULL, 'd'},
+ {"extension", required_argument, NULL, 'e'},
{"file", required_argument, NULL, 'f'},
{"format", required_argument, NULL, 'F'},
{"host", required_argument, NULL, 'h'},
@@ -424,7 +432,7 @@ main(int argc, char **argv)
InitDumpOptions(&dopt);
- while ((c = getopt_long(argc, argv, "abBcCd:E:f:F:h:j:n:N:Op:RsS:t:T:U:vwWxZ:",
+ while ((c = getopt_long(argc, argv, "abBcCd:e:E:f:F:h:j:n:N:Op:RsS:t:T:U:vwWxZ:",
long_options, &optindex)) != -1)
{
switch (c)
@@ -453,6 +461,11 @@ main(int argc, char **argv)
dopt.cparams.dbname = pg_strdup(optarg);
break;
+ case 'e': /* include extension(s) */
+ simple_string_list_append(&extension_include_patterns, optarg);
+ dopt.include_everything = false;
+ break;
+
case 'E': /* Dump encoding */
dumpencoding = pg_strdup(optarg);
break;
@@ -832,6 +845,16 @@ main(int argc, char **argv)
/* non-matching exclusion patterns aren't an error */
+ /* Expand extension selection patterns into OID lists */
+ if (extension_include_patterns.head != NULL)
+ {
+ expand_extension_name_patterns(fout, &extension_include_patterns,
+ &extension_include_oids,
+ strict_names);
+ if (extension_include_oids.head == NULL)
+ fatal("no matching extensions were found");
+ }
+
/*
* Dumping blobs is the default for dumps where an inclusion switch is not
* used (an "include everything" dump). -B can be used to exclude blobs
@@ -1019,6 +1042,7 @@ help(const char *progname)
printf(_(" -B, --no-blobs exclude large objects in dump\n"));
printf(_(" -c, --clean clean (drop) database objects before recreating\n"));
printf(_(" -C, --create include commands to create database in dump\n"));
+ printf(_(" -e, --extension=PATTERN dump the specified extension(s) only\n"));
printf(_(" -E, --encoding=ENCODING dump the data in encoding ENCODING\n"));
printf(_(" -n, --schema=PATTERN dump the specified schema(s) only\n"));
printf(_(" -N, --exclude-schema=PATTERN do NOT dump the specified schema(s)\n"));
@@ -1360,6 +1384,54 @@ expand_schema_name_patterns(Archive *fout,
destroyPQExpBuffer(query);
}
+/*
+ * Find the OIDs of all extensions matching the given list of patterns,
+ * and append them to the given OID list.
+ */
+static void
+expand_extension_name_patterns(Archive *fout,
+ SimpleStringList *patterns,
+ SimpleOidList *oids,
+ bool strict_names)
+{
+ PQExpBuffer query;
+ PGresult *res;
+ SimpleStringListCell *cell;
+ int i;
+
+ if (patterns->head == NULL)
+ return; /* nothing to do */
+
+ query = createPQExpBuffer();
+
+ /*
+ * The loop below runs multiple SELECTs might sometimes result in
+ * duplicate entries in the OID list, but we don't care.
+ */
+
+ for (cell = patterns->head; cell; cell = cell->next)
+ {
+ appendPQExpBufferStr(query,
+ "SELECT oid FROM pg_catalog.pg_extension e\n");
+ processSQLNamePattern(GetConnection(fout), query, cell->val, false,
+ false, NULL, "e.extname", NULL, NULL);
+
+ res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+ if (strict_names && PQntuples(res) == 0)
+ fatal("no matching extensions were found for pattern \"%s\"", cell->val);
+
+ for (i = 0; i < PQntuples(res); i++)
+ {
+ simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
+ }
+
+ PQclear(res);
+ resetPQExpBuffer(query);
+ }
+
+ destroyPQExpBuffer(query);
+}
+
/*
* Find the OIDs of all foreign servers matching the given list of patterns,
* and append them to the given OID list.
@@ -1799,6 +1871,11 @@ selectDumpableExtension(ExtensionInfo *extinfo, DumpOptions *dopt)
*/
if (extinfo->dobj.catId.oid <= (Oid) g_last_builtin_oid)
extinfo->dobj.dump = extinfo->dobj.dump_contains = DUMP_COMPONENT_ACL;
+ else if (extension_include_oids.head != NULL)
+ extinfo->dobj.dump_contains = extinfo->dobj.dump =
+ simple_oid_list_member(&extension_include_oids,
+ extinfo->dobj.catId.oid) ?
+ DUMP_COMPONENT_ALL : DUMP_COMPONENT_NONE;
else
extinfo->dobj.dump = extinfo->dobj.dump_contains =
dopt->include_everything ? DUMP_COMPONENT_ALL :
On Mon, Jan 25, 2021 at 9:34 PM Guillaume Lelarge
<guillaume@lelarge.info> wrote:
"Anytime soon" was a long long time ago, and I eventually completely forgot this, sorry. As nobody worked on it yet, I took a shot at it. See attached patch.
Great!
I didn't reviewed it thoroughly yet, but after a quick look it sounds
sensible. I'd prefer to see some tests added, and it looks like a
test for plpgsql could be added quite easily.
I don't know if I should add this right away in the commit fest app. If yes, I guess it should go on the next commit fest (2021-03), right?
Correct, and please add it on the commit fest!
Le mar. 26 janv. 2021 à 05:10, Julien Rouhaud <rjuju123@gmail.com> a écrit :
On Mon, Jan 25, 2021 at 9:34 PM Guillaume Lelarge
<guillaume@lelarge.info> wrote:"Anytime soon" was a long long time ago, and I eventually completely
forgot this, sorry. As nobody worked on it yet, I took a shot at it. See
attached patch.Great!
I didn't reviewed it thoroughly yet, but after a quick look it sounds
sensible. I'd prefer to see some tests added, and it looks like a
test for plpgsql could be added quite easily.
I tried that all afternoon yesterday, but failed to do so. My had still
hurts, but I'll try again though it may take some time.
I don't know if I should add this right away in the commit fest app. If
yes, I guess it should go on the next commit fest (2021-03), right?Correct, and please add it on the commit fest!
Done, see https://commitfest.postgresql.org/32/2956/.
--
Guillaume.
Le mar. 26 janv. 2021 à 13:41, Guillaume Lelarge <guillaume@lelarge.info> a
écrit :
Le mar. 26 janv. 2021 à 05:10, Julien Rouhaud <rjuju123@gmail.com> a
écrit :On Mon, Jan 25, 2021 at 9:34 PM Guillaume Lelarge
<guillaume@lelarge.info> wrote:"Anytime soon" was a long long time ago, and I eventually completely
forgot this, sorry. As nobody worked on it yet, I took a shot at it. See
attached patch.Great!
I didn't reviewed it thoroughly yet, but after a quick look it sounds
sensible. I'd prefer to see some tests added, and it looks like a
test for plpgsql could be added quite easily.I tried that all afternoon yesterday, but failed to do so. My had still
hurts, but I'll try again though it may take some time.
s/My had/My head/ ..
I don't know if I should add this right away in the commit fest app. If
yes, I guess it should go on the next commit fest (2021-03), right?
Correct, and please add it on the commit fest!
--
Guillaume.
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: not tested
The patch applies cleanly and looks fine to me. However consider this scenario.
- CREATE SCHEMA foo;
- CREATE EXTENSION file_fdw WITH SCHEMA foo;
- pg_dump --file=/tmp/test.sql --exclude-schema=foo postgres
This will still include the extension 'file_fdw' in the backup script. Shouldn't it be excluded as well?
The new status of this patch is: Waiting on Author
Hi,
Thanks for the review.
Le mer. 3 févr. 2021 à 18:33, Asif Rehman <asifr.rehman@gmail.com> a écrit :
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: not testedThe patch applies cleanly and looks fine to me. However consider this
scenario.- CREATE SCHEMA foo;
- CREATE EXTENSION file_fdw WITH SCHEMA foo;
- pg_dump --file=/tmp/test.sql --exclude-schema=foo postgresThis will still include the extension 'file_fdw' in the backup script.
Shouldn't it be excluded as well?The new status of this patch is: Waiting on Author
This behaviour is already there without my patch, and I think it's a valid
behaviour. An extension doesn't belong to a schema. Its objects do, but the
extension doesn't.
--
Guillaume.
Le mar. 26 janv. 2021 à 13:42, Guillaume Lelarge <guillaume@lelarge.info> a
écrit :
Le mar. 26 janv. 2021 à 13:41, Guillaume Lelarge <guillaume@lelarge.info>
a écrit :Le mar. 26 janv. 2021 à 05:10, Julien Rouhaud <rjuju123@gmail.com> a
écrit :On Mon, Jan 25, 2021 at 9:34 PM Guillaume Lelarge
<guillaume@lelarge.info> wrote:"Anytime soon" was a long long time ago, and I eventually completely
forgot this, sorry. As nobody worked on it yet, I took a shot at it. See
attached patch.Great!
I didn't reviewed it thoroughly yet, but after a quick look it sounds
sensible. I'd prefer to see some tests added, and it looks like a
test for plpgsql could be added quite easily.I tried that all afternoon yesterday, but failed to do so. My had still
hurts, but I'll try again though it may take some time.s/My had/My head/ ..
I finally managed to get a working TAP test for my patch. I have no idea if
it's good, and if it's enough. Anyway, new version of the patch attached.
--
Guillaume.
Attachments:
dump_extensions_v2.patchtext/x-patch; charset=US-ASCII; name=dump_extensions_v2.patchDownload
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index bcbb7a25fb..95d45fabfb 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -215,6 +215,38 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>-e <replaceable class="parameter">pattern</replaceable></option></term>
+ <term><option>--extension=<replaceable class="parameter">pattern</replaceable></option></term>
+ <listitem>
+ <para>
+ Dump only extensions matching <replaceable
+ class="parameter">pattern</replaceable>. When this option is not
+ specified, all non-system extensions in the target database will be
+ dumped. Multiple schemas can be selected by writing multiple
+ <option>-e</option> switches. The <replaceable
+ class="parameter">pattern</replaceable> parameter is interpreted as a
+ pattern according to the same rules used by
+ <application>psql</application>'s <literal>\d</literal> commands (see
+ <xref linkend="app-psql-patterns"/>), so multiple extensions can also
+ be selected by writing wildcard characters in the pattern. When using
+ wildcards, be careful to quote the pattern if needed to prevent the
+ shell from expanding the wildcards.
+ </para>
+
+ <note>
+ <para>
+ When <option>-e</option> is specified,
+ <application>pg_dump</application> makes no attempt to dump any other
+ database objects that the selected extension(s) might depend upon.
+ Therefore, there is no guarantee that the results of a
+ specific-extension dump can be successfully restored by themselves
+ into a clean database.
+ </para>
+ </note>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>-E <replaceable class="parameter">encoding</replaceable></option></term>
<term><option>--encoding=<replaceable class="parameter">encoding</replaceable></option></term>
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index eb988d7eb4..59c4e756ef 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -123,6 +123,9 @@ static SimpleOidList tabledata_exclude_oids = {NULL, NULL};
static SimpleStringList foreign_servers_include_patterns = {NULL, NULL};
static SimpleOidList foreign_servers_include_oids = {NULL, NULL};
+static SimpleStringList extension_include_patterns = {NULL, NULL};
+static SimpleOidList extension_include_oids = {NULL, NULL};
+
static const CatalogId nilCatalogId = {0, 0};
/* override for standard extra_float_digits setting */
@@ -151,6 +154,10 @@ static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids,
bool strict_names);
+static void expand_extension_name_patterns(Archive *fout,
+ SimpleStringList *patterns,
+ SimpleOidList *oids,
+ bool strict_names);
static void expand_foreign_server_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids);
@@ -334,6 +341,7 @@ main(int argc, char **argv)
{"clean", no_argument, NULL, 'c'},
{"create", no_argument, NULL, 'C'},
{"dbname", required_argument, NULL, 'd'},
+ {"extension", required_argument, NULL, 'e'},
{"file", required_argument, NULL, 'f'},
{"format", required_argument, NULL, 'F'},
{"host", required_argument, NULL, 'h'},
@@ -424,7 +432,7 @@ main(int argc, char **argv)
InitDumpOptions(&dopt);
- while ((c = getopt_long(argc, argv, "abBcCd:E:f:F:h:j:n:N:Op:RsS:t:T:U:vwWxZ:",
+ while ((c = getopt_long(argc, argv, "abBcCd:e:E:f:F:h:j:n:N:Op:RsS:t:T:U:vwWxZ:",
long_options, &optindex)) != -1)
{
switch (c)
@@ -453,6 +461,11 @@ main(int argc, char **argv)
dopt.cparams.dbname = pg_strdup(optarg);
break;
+ case 'e': /* include extension(s) */
+ simple_string_list_append(&extension_include_patterns, optarg);
+ dopt.include_everything = false;
+ break;
+
case 'E': /* Dump encoding */
dumpencoding = pg_strdup(optarg);
break;
@@ -832,6 +845,16 @@ main(int argc, char **argv)
/* non-matching exclusion patterns aren't an error */
+ /* Expand extension selection patterns into OID lists */
+ if (extension_include_patterns.head != NULL)
+ {
+ expand_extension_name_patterns(fout, &extension_include_patterns,
+ &extension_include_oids,
+ strict_names);
+ if (extension_include_oids.head == NULL)
+ fatal("no matching extensions were found");
+ }
+
/*
* Dumping blobs is the default for dumps where an inclusion switch is not
* used (an "include everything" dump). -B can be used to exclude blobs
@@ -1019,6 +1042,7 @@ help(const char *progname)
printf(_(" -B, --no-blobs exclude large objects in dump\n"));
printf(_(" -c, --clean clean (drop) database objects before recreating\n"));
printf(_(" -C, --create include commands to create database in dump\n"));
+ printf(_(" -e, --extension=PATTERN dump the specified extension(s) only\n"));
printf(_(" -E, --encoding=ENCODING dump the data in encoding ENCODING\n"));
printf(_(" -n, --schema=PATTERN dump the specified schema(s) only\n"));
printf(_(" -N, --exclude-schema=PATTERN do NOT dump the specified schema(s)\n"));
@@ -1360,6 +1384,54 @@ expand_schema_name_patterns(Archive *fout,
destroyPQExpBuffer(query);
}
+/*
+ * Find the OIDs of all extensions matching the given list of patterns,
+ * and append them to the given OID list.
+ */
+static void
+expand_extension_name_patterns(Archive *fout,
+ SimpleStringList *patterns,
+ SimpleOidList *oids,
+ bool strict_names)
+{
+ PQExpBuffer query;
+ PGresult *res;
+ SimpleStringListCell *cell;
+ int i;
+
+ if (patterns->head == NULL)
+ return; /* nothing to do */
+
+ query = createPQExpBuffer();
+
+ /*
+ * The loop below runs multiple SELECTs might sometimes result in
+ * duplicate entries in the OID list, but we don't care.
+ */
+
+ for (cell = patterns->head; cell; cell = cell->next)
+ {
+ appendPQExpBufferStr(query,
+ "SELECT oid FROM pg_catalog.pg_extension e\n");
+ processSQLNamePattern(GetConnection(fout), query, cell->val, false,
+ false, NULL, "e.extname", NULL, NULL);
+
+ res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+ if (strict_names && PQntuples(res) == 0)
+ fatal("no matching extensions were found for pattern \"%s\"", cell->val);
+
+ for (i = 0; i < PQntuples(res); i++)
+ {
+ simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
+ }
+
+ PQclear(res);
+ resetPQExpBuffer(query);
+ }
+
+ destroyPQExpBuffer(query);
+}
+
/*
* Find the OIDs of all foreign servers matching the given list of patterns,
* and append them to the given OID list.
@@ -1799,6 +1871,11 @@ selectDumpableExtension(ExtensionInfo *extinfo, DumpOptions *dopt)
*/
if (extinfo->dobj.catId.oid <= (Oid) g_last_builtin_oid)
extinfo->dobj.dump = extinfo->dobj.dump_contains = DUMP_COMPONENT_ACL;
+ else if (extension_include_oids.head != NULL)
+ extinfo->dobj.dump_contains = extinfo->dobj.dump =
+ simple_oid_list_member(&extension_include_oids,
+ extinfo->dobj.catId.oid) ?
+ DUMP_COMPONENT_ALL : DUMP_COMPONENT_NONE;
else
extinfo->dobj.dump = extinfo->dobj.dump_contains =
dopt->include_everything ? DUMP_COMPONENT_ALL :
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 737e46464a..49dc102d3d 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -340,6 +340,12 @@ my %pgdump_runs = (
'--schema=dump_test', '-b', '-B', '--no-sync', 'postgres',
],
+ },
+ test_schema_plus_extensions => {
+ dump_cmd => [
+ 'pg_dump', "--file=$tempdir/test_schema_plus_extensions.sql",
+ '--schema=dump_test', '--extension=plpgsql', '--no-sync', 'postgres',
+ ],
},);
###############################################################
@@ -381,7 +387,8 @@ my %pgdump_runs = (
# Tests which target the 'dump_test' schema, specifically.
my %dump_test_schema_runs = (
only_dump_test_schema => 1,
- test_schema_plus_blobs => 1,);
+ test_schema_plus_blobs => 1,
+ test_schema_plus_extensions => 1,);
# Tests which are considered 'full' dumps by pg_dump, but there
# are flags used to exclude specific items (ACLs, blobs, etc).
@@ -2680,6 +2687,7 @@ my %tests = (
schema_only => 1,
section_post_data => 1,
test_schema_plus_blobs => 1,
+ test_schema_plus_extensions => 1,
},
unlike => {
exclude_dump_test_schema => 1,
On Thu, Feb 18, 2021 at 11:13:06AM +0100, Guillaume Lelarge wrote:
I finally managed to get a working TAP test for my patch. I have no idea if
it's good, and if it's enough. Anyway, new version of the patch attached.
As presented in this patch, specifying both --extension and
--table/--schema means that pg_dump will dump both tables and
extensions matching the pattern passed down. But shouldn't extensions
not be dumped if --table or --schema is used? Combining --schema with
--table implies that the schema part is ignored, for instance.
The comment at the top of selectDumpableExtension() is incorrect,
missing the new case added by --extension.
The use of strict_names looks right to me, but you have missed a
refresh of the documentation of --strict-names that applies to
--extension with this patch.
+ dump_cmd => [
+ 'pg_dump', "--file=$tempdir/test_schema_plus_extensions.sql",
+ '--schema=dump_test', '--extension=plpgsql', '--no-sync',
What's the goal of this test. plpgsql is a system extension so it
will never be dumped. It seems to me that any tests related to this
patch should be added to src/test/modules/test_pg_dump/, which
includes a custom extension in the test, that could be dumped.+ dumped. Multiple schemas can be selected by writing multiple
+ <option>-e</option> switches. The <replaceable
s/schemas/extensions/.
--
MichaelMichael Paquier <michael@paquier.xyz> writes:
As presented in this patch, specifying both --extension and
--table/--schema means that pg_dump will dump both tables and
extensions matching the pattern passed down. But shouldn't extensions
not be dumped if --table or --schema is used? Combining --schema with
--table implies that the schema part is ignored, for instance.
I haven't read the patch, but the behavior I would expect is:
1. If --extension=pattern is given, then extensions matching the
pattern are included in the dump, regardless of other switches.
(Conversely, use of --extension doesn't affect choices about what
other objects are dumped.)
2. Without --extension, the behavior is backward compatible,
ie, dump extensions in an include_everything dump but not
otherwise.
Maybe we could have a separate discussion as to which switches turn
off include_everything, but that seems independent of this patch.
regards, tom lane
Le sam. 20 févr. 2021 à 13:25, Michael Paquier <michael@paquier.xyz> a
écrit :
On Thu, Feb 18, 2021 at 11:13:06AM +0100, Guillaume Lelarge wrote:
I finally managed to get a working TAP test for my patch. I have no idea
if
it's good, and if it's enough. Anyway, new version of the patch attached.
As presented in this patch, specifying both --extension and
--table/--schema means that pg_dump will dump both tables and
extensions matching the pattern passed down. But shouldn't extensions
not be dumped if --table or --schema is used? Combining --schema with
--table implies that the schema part is ignored, for instance.
Actually, that's the whole point of the patch. Imagine someone who wants to
backup a table with a citext column. With the --table option, this user
will only have the table, without the extension that would allow to restore
the dump. He will have to remember to create the extension before. The new
option allows him to add the CREATE EXTENSION he needs into his dump.
The comment at the top of selectDumpableExtension() is incorrect,
missing the new case added by --extension.
The use of strict_names looks right to me, but you have missed a
refresh of the documentation of --strict-names that applies to
--extension with this patch.+ dump_cmd => [ + 'pg_dump', "--file=$tempdir/test_schema_plus_extensions.sql", + '--schema=dump_test', '--extension=plpgsql', '--no-sync', What's the goal of this test. plpgsql is a system extension so it will never be dumped. It seems to me that any tests related to this patch should be added to src/test/modules/test_pg_dump/, which includes a custom extension in the test, that could be dumped.+ dumped. Multiple schemas can be selected by writing multiple + <option>-e</option> switches. The <replaceable s/schemas/extensions/.
You're right on all these points. I'm on vacation this week, but I'll build
a v3 with these issues fixed when I'll get back from vacation.
Thanks.
--
Guillaume.
Le sam. 20 févr. 2021 à 17:31, Tom Lane <tgl@sss.pgh.pa.us> a écrit :
Michael Paquier <michael@paquier.xyz> writes:
As presented in this patch, specifying both --extension and
--table/--schema means that pg_dump will dump both tables and
extensions matching the pattern passed down. But shouldn't extensions
not be dumped if --table or --schema is used? Combining --schema with
--table implies that the schema part is ignored, for instance.I haven't read the patch, but the behavior I would expect is:
1. If --extension=pattern is given, then extensions matching the
pattern are included in the dump, regardless of other switches.
(Conversely, use of --extension doesn't affect choices about what
other objects are dumped.)2. Without --extension, the behavior is backward compatible,
ie, dump extensions in an include_everything dump but not
otherwise.
Yes, that's what it's supposed to do.
Maybe we could have a separate discussion as to which switches turn
off include_everything, but that seems independent of this patch.
+1
--
Guillaume.
On Sat, Feb 20, 2021 at 10:39:24PM +0100, Guillaume Lelarge wrote:
Le sam. 20 févr. 2021 à 17:31, Tom Lane <tgl@sss.pgh.pa.us> a écrit :
I haven't read the patch, but the behavior I would expect is:
1. If --extension=pattern is given, then extensions matching the
pattern are included in the dump, regardless of other switches.
(Conversely, use of --extension doesn't affect choices about what
other objects are dumped.)2. Without --extension, the behavior is backward compatible,
ie, dump extensions in an include_everything dump but not
otherwise.Yes, that's what it's supposed to do.
Okay, that sounds fine to me. Thanks for confirming.
--
Michael
On Thu, Feb 18, 2021 at 11:13:06AM +0100, Guillaume Lelarge wrote:
Le mar. 26 janv. 2021 � 13:42, Guillaume Lelarge <guillaume@lelarge.info> a
�crit :Le mar. 26 janv. 2021 � 13:41, Guillaume Lelarge <guillaume@lelarge.info>
a �crit :Le mar. 26 janv. 2021 � 05:10, Julien Rouhaud <rjuju123@gmail.com> a
�crit :On Mon, Jan 25, 2021 at 9:34 PM Guillaume Lelarge
<guillaume@lelarge.info> wrote:"Anytime soon" was a long long time ago, and I eventually completely
forgot this, sorry. As nobody worked on it yet, I took a shot at it. See
attached patch.Great!
I didn't reviewed it thoroughly yet, but after a quick look it sounds
sensible. I'd prefer to see some tests added, and it looks like a
test for plpgsql could be added quite easily.I tried that all afternoon yesterday, but failed to do so. My had still
hurts, but I'll try again though it may take some time.s/My had/My head/ ..
I finally managed to get a working TAP test for my patch. I have no idea if
it's good, and if it's enough. Anyway, new version of the patch attached.--
Guillaume.
Thanks for doing this work!
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml index bcbb7a25fb..95d45fabfb 100644 --- a/doc/src/sgml/ref/pg_dump.sgml +++ b/doc/src/sgml/ref/pg_dump.sgml @@ -215,6 +215,38 @@ PostgreSQL documentation </listitem> </varlistentry>+ <varlistentry> + <term><option>-e <replaceable class="parameter">pattern</replaceable></option></term> + <term><option>--extension=<replaceable class="parameter">pattern</replaceable></option></term> + <listitem> + <para> + Dump only extensions matching <replaceable + class="parameter">pattern</replaceable>. When this option is not + specified, all non-system extensions in the target database will be + dumped. Multiple schemas can be selected by writing multiple
^^^^^^^^^^^^^^^^
I think this should read "Multiple extensions".
Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
On Sun, Feb 21, 2021 at 08:14:45AM +0900, Michael Paquier wrote:
Okay, that sounds fine to me. Thanks for confirming.
Guillaume, it has been a couple of weeks since your last update. Are
you planning to send a new version of the patch?
--
Michael
Le lun. 15 mars 2021 à 06:00, Michael Paquier <michael@paquier.xyz> a
écrit :
On Sun, Feb 21, 2021 at 08:14:45AM +0900, Michael Paquier wrote:
Okay, that sounds fine to me. Thanks for confirming.
Guillaume, it has been a couple of weeks since your last update. Are
you planning to send a new version of the patch?
This is on my TODO list, but I can't find the time right now. If someone's
interested in this patch, i have no problem with him/her working on it.
Otherwise, I'll do it as soon as possible (meaning probably in two weeks).
On Mon, Mar 15, 2021 at 09:19:19AM +0100, Guillaume Lelarge wrote:
Le lun. 15 mars 2021 � 06:00, Michael Paquier <michael@paquier.xyz> a
�crit :On Sun, Feb 21, 2021 at 08:14:45AM +0900, Michael Paquier wrote:
Okay, that sounds fine to me. Thanks for confirming.
Guillaume, it has been a couple of weeks since your last update. Are
you planning to send a new version of the patch?This is on my TODO list, but I can't find the time right now. If someone's
interested in this patch, i have no problem with him/her working on it.
Otherwise, I'll do it as soon as possible (meaning probably in two weeks).
That will (almost) be the end of the last commitfest for pg14.
Is that a feature you really want to see in pg14? If yes and if you're sure
you won't have time to work on the patch within 2 weeks I can take care of
addressing all comments.
On Mon, Mar 15, 2021 at 06:21:55PM +0800, Julien Rouhaud wrote:
Is that a feature you really want to see in pg14? If yes and if you're sure
you won't have time to work on the patch within 2 weeks I can take care of
addressing all comments.
A lot of things will depend on the feature freeze date, but the sooner
a version is available, the sooner I could look at what is proposed.
Recalling my memories of the discussion, there was not much to
address, and the logic was rather clear.
--
Michael
Le lun. 15 mars 2021 à 18:25, Michael Paquier <michael@paquier.xyz> a
écrit :
On Mon, Mar 15, 2021 at 06:21:55PM +0800, Julien Rouhaud wrote:
Is that a feature you really want to see in pg14? If yes and if you're
sure
you won't have time to work on the patch within 2 weeks I can take care
of
addressing all comments.
A lot of things will depend on the feature freeze date, but the sooner
a version is available, the sooner I could look at what is proposed.
I totally agree.
Recalling my memories of the discussion, there was not much to
address, and the logic was rather clear.
yes, I just had a look it's only a matter of adapting the tests to
test_pg_dump and fixing a few typos AFAICS.
Show quoted text
Le lun. 15 mars 2021 à 11:32, Julien Rouhaud <rjuju123@gmail.com> a écrit :
Le lun. 15 mars 2021 à 18:25, Michael Paquier <michael@paquier.xyz> a
écrit :On Mon, Mar 15, 2021 at 06:21:55PM +0800, Julien Rouhaud wrote:
Is that a feature you really want to see in pg14? If yes and if you're
sure
you won't have time to work on the patch within 2 weeks I can take care
of
addressing all comments.
A lot of things will depend on the feature freeze date, but the sooner
a version is available, the sooner I could look at what is proposed.I totally agree.
Recalling my memories of the discussion, there was not much to
address, and the logic was rather clear.
yes, I just had a look it's only a matter of adapting the tests to
test_pg_dump and fixing a few typos AFAICS.
Jeez, you're answering too fast. I was working on my own answer when Julien
sent another reply 😅
Anyways. Yeah, I know we're near feature freeze. This feature would be nice
to have, but I don't feel strongly about it. I think this feature is
currently lacking in PostgreSQL but I don't much care if it makes it to 14
or any future release. If you have time to work on the pg_dump test suite
and are interested, then sure, go ahead, I'm fine with this. Otherwise I'll
do it in a few weeks and if it means it'll land in v15, then be it. That's
not an issue in itself.
Though, I really appreciate the concern. Thanks.
Show quoted text
On Mon, Mar 15, 2021 at 11:37:02AM +0100, Guillaume Lelarge wrote:
Anyways. Yeah, I know we're near feature freeze. This feature would be nice
to have, but I don't feel strongly about it. I think this feature is
currently lacking in PostgreSQL but I don't much care if it makes it to 14
or any future release. If you have time to work on the pg_dump test suite
and are interested, then sure, go ahead, I'm fine with this. Otherwise I'll
do it in a few weeks and if it means it'll land in v15, then be it. That's
not an issue in itself.
Okay. So I have looked at that stuff in details, and after fixing
all the issues reported upthread in the code, docs and tests I am
finishing with the attached. The tests have been moved out of
src/bin/pg_dump/ to src/test/modules/test_pg_dump/, and include both
positive and negative tests (used the trick with plpgsql for the
latter to avoid the dump of the extension test_pg_dump or any data
related to it).
--
Michael
Attachments:
0001-Add-support-for-extension-in-pg_dump.patchtext/x-diff; charset=us-asciiDownload
From ca1388a4c4441f15c02d332b8a9996408a90662c Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Tue, 30 Mar 2021 12:01:13 +0900
Subject: [PATCH] Add support for --extension in pg_dump
---
src/bin/pg_dump/pg_dump.c | 93 +++++++++++++++++++--
src/test/modules/test_pg_dump/t/001_base.pl | 53 ++++++++----
doc/src/sgml/ref/pg_dump.sgml | 43 ++++++++--
3 files changed, 161 insertions(+), 28 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index da6cc054b0..382d33f2c6 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -123,6 +123,9 @@ static SimpleOidList tabledata_exclude_oids = {NULL, NULL};
static SimpleStringList foreign_servers_include_patterns = {NULL, NULL};
static SimpleOidList foreign_servers_include_oids = {NULL, NULL};
+static SimpleStringList extension_include_patterns = {NULL, NULL};
+static SimpleOidList extension_include_oids = {NULL, NULL};
+
static const CatalogId nilCatalogId = {0, 0};
/* override for standard extra_float_digits setting */
@@ -151,6 +154,10 @@ static void expand_schema_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids,
bool strict_names);
+static void expand_extension_name_patterns(Archive *fout,
+ SimpleStringList *patterns,
+ SimpleOidList *oids,
+ bool strict_names);
static void expand_foreign_server_name_patterns(Archive *fout,
SimpleStringList *patterns,
SimpleOidList *oids);
@@ -335,6 +342,7 @@ main(int argc, char **argv)
{"clean", no_argument, NULL, 'c'},
{"create", no_argument, NULL, 'C'},
{"dbname", required_argument, NULL, 'd'},
+ {"extension", required_argument, NULL, 'e'},
{"file", required_argument, NULL, 'f'},
{"format", required_argument, NULL, 'F'},
{"host", required_argument, NULL, 'h'},
@@ -426,7 +434,7 @@ main(int argc, char **argv)
InitDumpOptions(&dopt);
- while ((c = getopt_long(argc, argv, "abBcCd:E:f:F:h:j:n:N:Op:RsS:t:T:U:vwWxZ:",
+ while ((c = getopt_long(argc, argv, "abBcCd:e:E:f:F:h:j:n:N:Op:RsS:t:T:U:vwWxZ:",
long_options, &optindex)) != -1)
{
switch (c)
@@ -455,6 +463,11 @@ main(int argc, char **argv)
dopt.cparams.dbname = pg_strdup(optarg);
break;
+ case 'e': /* include extension(s) */
+ simple_string_list_append(&extension_include_patterns, optarg);
+ dopt.include_everything = false;
+ break;
+
case 'E': /* Dump encoding */
dumpencoding = pg_strdup(optarg);
break;
@@ -834,6 +847,16 @@ main(int argc, char **argv)
/* non-matching exclusion patterns aren't an error */
+ /* Expand extension selection patterns into OID lists */
+ if (extension_include_patterns.head != NULL)
+ {
+ expand_extension_name_patterns(fout, &extension_include_patterns,
+ &extension_include_oids,
+ strict_names);
+ if (extension_include_oids.head == NULL)
+ fatal("no matching extensions were found");
+ }
+
/*
* Dumping blobs is the default for dumps where an inclusion switch is not
* used (an "include everything" dump). -B can be used to exclude blobs
@@ -1025,6 +1048,7 @@ help(const char *progname)
printf(_(" -B, --no-blobs exclude large objects in dump\n"));
printf(_(" -c, --clean clean (drop) database objects before recreating\n"));
printf(_(" -C, --create include commands to create database in dump\n"));
+ printf(_(" -e, --extension=PATTERN dump the specified extension(s) only\n"));
printf(_(" -E, --encoding=ENCODING dump the data in encoding ENCODING\n"));
printf(_(" -n, --schema=PATTERN dump the specified schema(s) only\n"));
printf(_(" -N, --exclude-schema=PATTERN do NOT dump the specified schema(s)\n"));
@@ -1367,6 +1391,53 @@ expand_schema_name_patterns(Archive *fout,
destroyPQExpBuffer(query);
}
+/*
+ * Find the OIDs of all extensions matching the given list of patterns,
+ * and append them to the given OID list.
+ */
+static void
+expand_extension_name_patterns(Archive *fout,
+ SimpleStringList *patterns,
+ SimpleOidList *oids,
+ bool strict_names)
+{
+ PQExpBuffer query;
+ PGresult *res;
+ SimpleStringListCell *cell;
+ int i;
+
+ if (patterns->head == NULL)
+ return; /* nothing to do */
+
+ query = createPQExpBuffer();
+
+ /*
+ * The loop below runs multiple SELECTs might sometimes result in
+ * duplicate entries in the OID list, but we don't care.
+ */
+ for (cell = patterns->head; cell; cell = cell->next)
+ {
+ appendPQExpBufferStr(query,
+ "SELECT oid FROM pg_catalog.pg_extension e\n");
+ processSQLNamePattern(GetConnection(fout), query, cell->val, false,
+ false, NULL, "e.extname", NULL, NULL);
+
+ res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+ if (strict_names && PQntuples(res) == 0)
+ fatal("no matching extensions were found for pattern \"%s\"", cell->val);
+
+ for (i = 0; i < PQntuples(res); i++)
+ {
+ simple_oid_list_append(oids, atooid(PQgetvalue(res, i, 0)));
+ }
+
+ PQclear(res);
+ resetPQExpBuffer(query);
+ }
+
+ destroyPQExpBuffer(query);
+}
+
/*
* Find the OIDs of all foreign servers matching the given list of patterns,
* and append them to the given OID list.
@@ -1793,8 +1864,9 @@ selectDumpableAccessMethod(AccessMethodInfo *method, Archive *fout)
* Built-in extensions should be skipped except for checking ACLs, since we
* assume those will already be installed in the target database. We identify
* such extensions by their having OIDs in the range reserved for initdb.
- * We dump all user-added extensions by default, or none of them if
- * include_everything is false (i.e., a --schema or --table switch was given).
+ * We dump all user-added extensions by default. No extensions are dumped
+ * if include_everything is false (i.e., a --schema or --table switch was
+ * given), except if --extension specifies a list of extensions to dump.
*/
static void
selectDumpableExtension(ExtensionInfo *extinfo, DumpOptions *dopt)
@@ -1807,9 +1879,18 @@ selectDumpableExtension(ExtensionInfo *extinfo, DumpOptions *dopt)
if (extinfo->dobj.catId.oid <= (Oid) g_last_builtin_oid)
extinfo->dobj.dump = extinfo->dobj.dump_contains = DUMP_COMPONENT_ACL;
else
- extinfo->dobj.dump = extinfo->dobj.dump_contains =
- dopt->include_everything ? DUMP_COMPONENT_ALL :
- DUMP_COMPONENT_NONE;
+ {
+ /* check if there is a list of extensions to dump */
+ if (extension_include_oids.head != NULL)
+ extinfo->dobj.dump = extinfo->dobj.dump_contains =
+ simple_oid_list_member(&extension_include_oids,
+ extinfo->dobj.catId.oid) ?
+ DUMP_COMPONENT_ALL : DUMP_COMPONENT_NONE;
+ else
+ extinfo->dobj.dump = extinfo->dobj.dump_contains =
+ dopt->include_everything ?
+ DUMP_COMPONENT_ALL : DUMP_COMPONENT_NONE;
+ }
}
/*
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 501aff0920..7c053c4e49 100644
--- a/src/test/modules/test_pg_dump/t/001_base.pl
+++ b/src/test/modules/test_pg_dump/t/001_base.pl
@@ -194,6 +194,20 @@ my %pgdump_runs = (
'pg_dump', '--no-sync', "--file=$tempdir/section_post_data.sql",
'--section=post-data', 'postgres',
],
+ },
+ with_extension => {
+ dump_cmd => [
+ 'pg_dump', '--no-sync', "--file=$tempdir/with_extension.sql",
+ '--extension=test_pg_dump', '--no-sync', 'postgres',
+ ],
+ },
+
+ # plgsql in the list blocks the dump of extension test_pg_dump
+ without_extension => {
+ dump_cmd => [
+ 'pg_dump', '--no-sync', "--file=$tempdir/without_extension.sql",
+ '--extension=plpgsql', '--no-sync', 'postgres',
+ ],
},);
###############################################################
@@ -228,14 +242,16 @@ my %pgdump_runs = (
# Tests which are considered 'full' dumps by pg_dump, but there
# are flags used to exclude specific items (ACLs, blobs, etc).
my %full_runs = (
- binary_upgrade => 1,
- clean => 1,
- clean_if_exists => 1,
- createdb => 1,
- defaults => 1,
- exclude_table => 1,
- no_privs => 1,
- no_owner => 1,);
+ binary_upgrade => 1,
+ clean => 1,
+ clean_if_exists => 1,
+ createdb => 1,
+ defaults => 1,
+ exclude_table => 1,
+ no_privs => 1,
+ no_owner => 1,
+ with_extension => 1,
+ without_extension => 1);
my %tests = (
'ALTER EXTENSION test_pg_dump' => {
@@ -261,7 +277,7 @@ my %tests = (
schema_only => 1,
section_pre_data => 1,
},
- unlike => { binary_upgrade => 1, },
+ unlike => { binary_upgrade => 1, without_extension => 1 },
},
'CREATE ROLE regress_dump_test_role' => {
@@ -320,6 +336,7 @@ my %tests = (
section_data => 1,
extension_schema => 1,
},
+ unlike => { without_extension => 1, },
},
'CREATE TABLE regress_pg_dump_table' => {
@@ -343,8 +360,9 @@ my %tests = (
extension_schema => 1,
},
unlike => {
- binary_upgrade => 1,
- exclude_table => 1,
+ binary_upgrade => 1,
+ exclude_table => 1,
+ without_extension => 1,
},
},
@@ -367,7 +385,7 @@ my %tests = (
schema_only => 1,
section_pre_data => 1,
},
- unlike => { no_privs => 1, },
+ unlike => { no_privs => 1, without_extension => 1, },
},
'REVOKE GRANT OPTION FOR UPDATE ON SEQUENCE wgo_then_regular' => {
@@ -384,7 +402,7 @@ my %tests = (
schema_only => 1,
section_pre_data => 1,
},
- unlike => { no_privs => 1, },
+ unlike => { no_privs => 1, without_extension => 1, },
},
'CREATE ACCESS METHOD regress_test_am' => {
@@ -404,6 +422,7 @@ my %tests = (
schema_only => 1,
section_pre_data => 1,
},
+ unlike => { without_extension => 1, },
},
'GRANT SELECT regress_pg_dump_table_added pre-ALTER EXTENSION' => {
@@ -428,7 +447,7 @@ my %tests = (
schema_only => 1,
section_pre_data => 1,
},
- unlike => { no_privs => 1, },
+ unlike => { no_privs => 1, without_extension => 1, },
},
'GRANT SELECT ON TABLE regress_pg_dump_table' => {
@@ -462,7 +481,7 @@ my %tests = (
schema_only => 1,
section_pre_data => 1,
},
- unlike => { no_privs => 1, },
+ unlike => { no_privs => 1, without_extension => 1 },
},
'GRANT USAGE ON regress_pg_dump_table_col1_seq TO regress_dump_test_role'
@@ -478,7 +497,7 @@ my %tests = (
schema_only => 1,
section_pre_data => 1,
},
- unlike => { no_privs => 1, },
+ unlike => { no_privs => 1, without_extension => 1, },
},
'GRANT USAGE ON regress_pg_dump_seq TO regress_dump_test_role' => {
@@ -500,7 +519,7 @@ my %tests = (
schema_only => 1,
section_pre_data => 1,
},
- unlike => { no_privs => 1, },
+ unlike => { no_privs => 1, without_extension => 1, },
},
# Objects included in extension part of a schema created by this extension */
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
index 989b8e2381..529b167c96 100644
--- a/doc/src/sgml/ref/pg_dump.sgml
+++ b/doc/src/sgml/ref/pg_dump.sgml
@@ -215,6 +215,38 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>-e <replaceable class="parameter">pattern</replaceable></option></term>
+ <term><option>--extension=<replaceable class="parameter">pattern</replaceable></option></term>
+ <listitem>
+ <para>
+ Dump only extensions matching <replaceable
+ class="parameter">pattern</replaceable>. When this option is not
+ specified, all non-system extensions in the target database will be
+ dumped. Multiple extensions can be selected by writing multiple
+ <option>-e</option> switches. The <replaceable
+ class="parameter">pattern</replaceable> parameter is interpreted as a
+ pattern according to the same rules used by
+ <application>psql</application>'s <literal>\d</literal> commands (see
+ <xref linkend="app-psql-patterns"/>), so multiple extensions can also
+ be selected by writing wildcard characters in the pattern. When using
+ wildcards, be careful to quote the pattern if needed to prevent the
+ shell from expanding the wildcards.
+ </para>
+
+ <note>
+ <para>
+ When <option>-e</option> is specified,
+ <application>pg_dump</application> makes no attempt to dump any other
+ database objects that the selected extension(s) might depend upon.
+ Therefore, there is no guarantee that the results of a
+ specific-extension dump can be successfully restored by themselves
+ into a clean database.
+ </para>
+ </note>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>-E <replaceable class="parameter">encoding</replaceable></option></term>
<term><option>--encoding=<replaceable class="parameter">encoding</replaceable></option></term>
@@ -1079,11 +1111,12 @@ PostgreSQL documentation
<term><option>--strict-names</option></term>
<listitem>
<para>
- Require that each schema
- (<option>-n</option>/<option>--schema</option>) and table
- (<option>-t</option>/<option>--table</option>) qualifier match at
- least one schema/table in the database to be dumped. Note that if
- none of the schema/table qualifiers find
+ Require that each
+ extension (<option>-e</option>/<option>--extension</option>),
+ schema (<option>-n</option>/<option>--schema</option>) and
+ table (<option>-t</option>/<option>--table</option>) qualifier
+ match at least one extension/schema/table in the database to be dumped.
+ Note that if none of the extension/schema/table qualifiers find
matches, <application>pg_dump</application> will generate an error
even without <option>--strict-names</option>.
</para>
--
2.31.0
On Tue, Mar 30, 2021 at 12:02:45PM +0900, Michael Paquier wrote:
Okay. So I have looked at that stuff in details, and after fixing
all the issues reported upthread in the code, docs and tests I am
finishing with the attached. The tests have been moved out of
src/bin/pg_dump/ to src/test/modules/test_pg_dump/, and include both
positive and negative tests (used the trick with plpgsql for the
latter to avoid the dump of the extension test_pg_dump or any data
related to it).
I have double-checked this stuff this morning, and did not notice any
issues. So, applied.
--
Michael
Le mer. 31 mars 2021 à 02:37, Michael Paquier <michael@paquier.xyz> a
écrit :
On Tue, Mar 30, 2021 at 12:02:45PM +0900, Michael Paquier wrote:
Okay. So I have looked at that stuff in details, and after fixing
all the issues reported upthread in the code, docs and tests I am
finishing with the attached. The tests have been moved out of
src/bin/pg_dump/ to src/test/modules/test_pg_dump/, and include both
positive and negative tests (used the trick with plpgsql for the
latter to avoid the dump of the extension test_pg_dump or any data
related to it).I have double-checked this stuff this morning, and did not notice any
issues. So, applied.
Thanks a lot. I've seen your email yesterday but had too much work going on
to find the time to test your patch. Anyway, I'll take a look at how you
coded the TAP test to better understand that part and to be able to do it
myself next time.
Thanks again.
On Wed, Mar 31, 2021 at 09:37:44AM +0900, Michael Paquier wrote:
On Tue, Mar 30, 2021 at 12:02:45PM +0900, Michael Paquier wrote:
Okay. So I have looked at that stuff in details, and after fixing
all the issues reported upthread in the code, docs and tests I am
finishing with the attached. The tests have been moved out of
src/bin/pg_dump/ to src/test/modules/test_pg_dump/, and include both
positive and negative tests (used the trick with plpgsql for the
latter to avoid the dump of the extension test_pg_dump or any data
related to it).I have double-checked this stuff this morning, and did not notice any
issues. So, applied.
I noticed the patch's behavior for relations that are members of non-dumped
extensions and are also registered using pg_extension_config_dump(). It
depends on the schema:
- If extschema='public', "pg_dump -e plpgsql" makes no mention of the
relations.
- 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.)
- If extschema is not any sort of built-in schema, "pg_dump -e plpgsql"
includes commands to dump the relation data. This surprised me.
I'm attaching a test case patch that demonstrates this. Is this behavior
intentional?
Attachments:
without_extension_explicit_schema-v0.patchtext/plain; charset=us-asciiDownload
commit d210c01 (demo-dumpext-public)
Author: Noah Misch <noah@leadboat.com>
AuthorDate: Thu Apr 1 17:36:13 2021 -0700
Commit: Noah Misch <noah@leadboat.com>
CommitDate: Thu Apr 1 17:36:13 2021 -0700
demo
---
src/test/modules/test_pg_dump/t/001_base.pl | 16 ++++++++++++++++
1 file changed, 16 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 7c053c4..600015e 100644
--- a/src/test/modules/test_pg_dump/t/001_base.pl
+++ b/src/test/modules/test_pg_dump/t/001_base.pl
@@ -203,11 +203,23 @@ my %pgdump_runs = (
},
# plgsql in the list blocks the dump of extension test_pg_dump
+ # TODO --no-sync needn't appear twice
without_extension => {
dump_cmd => [
'pg_dump', '--no-sync', "--file=$tempdir/without_extension.sql",
'--extension=plpgsql', '--no-sync', 'postgres',
],
+ },
+
+ without_extension_explicit_schema => {
+ dump_cmd => [
+ 'pg_dump',
+ '--no-sync',
+ "--file=$tempdir/without_extension_explicit_schema.sql",
+ '--extension=plpgsql',
+ '--schema=public',
+ 'postgres',
+ ],
},);
###############################################################
@@ -632,6 +644,8 @@ my %tests = (
pg_dumpall_globals => 1,
section_data => 1,
section_pre_data => 1,
+ # excludes this schema
+ without_extension_explicit_schema => 1,
},
},
@@ -646,6 +660,8 @@ my %tests = (
pg_dumpall_globals => 1,
section_data => 1,
section_pre_data => 1,
+ # excludes this schema
+ without_extension_explicit_schema => 1,
},
},
On Sun, Apr 04, 2021 at 03:08:02PM -0700, Noah Misch wrote:
On Wed, Mar 31, 2021 at 09:37:44AM +0900, Michael Paquier wrote:
On Tue, Mar 30, 2021 at 12:02:45PM +0900, Michael Paquier wrote:
Okay. So I have looked at that stuff in details, and after fixing
all the issues reported upthread in the code, docs and tests I am
finishing with the attached. The tests have been moved out of
src/bin/pg_dump/ to src/test/modules/test_pg_dump/, and include both
positive and negative tests (used the trick with plpgsql for the
latter to avoid the dump of the extension test_pg_dump or any data
related to it).I have double-checked this stuff this morning, and did not notice any
issues. So, applied.I noticed the patch's behavior for relations that are members of non-dumped
extensions and are also registered using pg_extension_config_dump(). It
depends on the schema:- If extschema='public', "pg_dump -e plpgsql" makes no mention of the
relations.
- 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.)
- If extschema is not any sort of built-in schema, "pg_dump -e plpgsql"
includes commands to dump the relation data. This surprised me.I'm attaching a test case patch that demonstrates this. Is this behavior
intentional?
I think this is a bug in $SUBJECT.
On Wed, Apr 07, 2021 at 07:42:11PM -0700, Noah Misch wrote:
I think this is a bug in $SUBJECT.
Sorry for the late reply. I intend to answer to that and this is
registered as an open item, but I got busy with some other things.
--
Michael
On Sun, Apr 04, 2021 at 03:08:02PM -0700, Noah Misch wrote:
I noticed the patch's behavior for relations that are members of non-dumped
extensions and are also registered using pg_extension_config_dump(). It
depends on the schema:- If extschema='public', "pg_dump -e plpgsql" makes no mention of the
relations.
This one is expected to me. The caller of pg_dump is not specifying
the extension that should be dumped, hence it looks logic to me to not
dump the tables marked as pg_extension_config_dump() part as an
extension not listed.
- 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. Anyway, the behavior to select if a schema can be dumped
or not is not really related to this new code, right? And "public" is
a mixed beast, being treated as a system object and a user object by
selectDumpableNamespace().
- If extschema is not any sort of built-in schema, "pg_dump -e plpgsql"
includes commands to dump the relation data. This surprised me.
Hmm. But you are right that this one is inconsistent with the first
case where the extension is not listed. I would have said that as the
extension is not directly specified and that the schema is not passed
down either then we should not dump it at all, but this combination
actually does so. Maybe we should add an extra logic into
selectDumpableNamespace(), close to the end of it, to decide if,
depending on the contents of the extensions to include, we should dump
its associated schema or not? Another solution would be to make use
of schema_include_oids to filter out the schemas we don't want, but
that would mean that --extension gets priority over --schema or
--table but we did ot want that per the original discussion.
--
Michael
On Tue, Apr 13, 2021 at 02:43:11PM +0900, Michael Paquier wrote:
On Sun, Apr 04, 2021 at 03:08:02PM -0700, Noah Misch wrote:
I noticed the patch's behavior for relations that are members of non-dumped
extensions and are also registered using pg_extension_config_dump(). It
depends on the schema:- If extschema='public', "pg_dump -e plpgsql" makes no mention of the
relations.This one is expected to me. The caller of pg_dump is not specifying
the extension that should be dumped, hence it looks logic to me to not
dump the tables marked as pg_extension_config_dump() part as an
extension not listed.
Agreed.
- 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.
Anyway, the behavior to select if a schema can be dumped
or not is not really related to this new code, right? And "public" is
a mixed beast, being treated as a system object and a user object by
selectDumpableNamespace().
Correct.
- If extschema is not any sort of built-in schema, "pg_dump -e plpgsql"
includes commands to dump the relation data. This surprised me.Hmm. But you are right that this one is inconsistent with the first
case where the extension is not listed. I would have said that as the
extension is not directly specified and that the schema is not passed
down either then we should not dump it at all, but this combination
actually does so. Maybe we should add an extra logic into
selectDumpableNamespace(), close to the end of it, to decide if,
depending on the contents of the extensions to include, we should dump
its associated schema or not? Another solution would be to make use
of schema_include_oids to filter out the schemas we don't want, but
that would mean that --extension gets priority over --schema or
--table but we did ot want that per the original discussion.
No, neither of those solutions apply. "pg_dump -e plpgsql" selects all
schemas. That is consistent with its documentation; plain "pg_dump" has long
selected all schemas, and the documentation for "-e" does not claim that "-e"
changes the selection of non-extension objects. We're not going to solve the
problem by making selectDumpableNamespace() select some additional aspect of
schema foo, because it's already selecting every available aspect. Like you
say, we're also not going to solve the problem by removing some existing
aspect of schema foo from selection, because that would remove dump material
unrelated to any extension.
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.
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
Attachments:
pgdump-extension-config.patchtext/x-diff; charset=us-asciiDownload
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,
On Wed, Apr 14, 2021 at 10:38:17AM +0900, Michael Paquier wrote:
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 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.
Yes.
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.
That's consistent with v13, and it seems fine. I've not used a non-test
extension that creates a schema.
--- 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', + ], },);
The name "without_extension_explicit_schema" arose because that test differs
from the "without_extension" test by adding --schema=public. The test named
"without_extension_implicit_schema" differs from "without_extension" by adding
--schema=regress_pg_dump_schema, so the word "implicit" feels not-descriptive
of the test. I recommend picking a different name. Other than that, the
change looks good.
On Wed, Apr 14, 2021 at 05:31:15AM -0700, Noah Misch wrote:
The name "without_extension_explicit_schema" arose because that test differs
from the "without_extension" test by adding --schema=public. The test named
"without_extension_implicit_schema" differs from "without_extension" by adding
--schema=regress_pg_dump_schema, so the word "implicit" feels not-descriptive
of the test. I recommend picking a different name. Other than that, the
change looks good.
Thanks for the review. I have picked up "internal" instead, as
that's the schema created within the extension itself, and applied the
patch.
--
Michael
Le jeu. 15 avr. 2021 à 09:58, Michael Paquier <michael@paquier.xyz> a
écrit :
On Wed, Apr 14, 2021 at 05:31:15AM -0700, Noah Misch wrote:
The name "without_extension_explicit_schema" arose because that test
differs
from the "without_extension" test by adding --schema=public. The test
named
"without_extension_implicit_schema" differs from "without_extension" by
adding
--schema=regress_pg_dump_schema, so the word "implicit" feels
not-descriptive
of the test. I recommend picking a different name. Other than that, the
change looks good.Thanks for the review. I have picked up "internal" instead, as
that's the schema created within the extension itself, and applied the
patch.
Thanks for the work on this. I didn't understand everything on the issue,
which is why I didn't say a thing, but I followed the thread, and very much
appreciated the fix.
--
Guillaume.