pg_dumpall --exclude-database option
PFA a patch to provide an --exclude-database option for pg_dumpall. The
causes pg_dumpall to skip any database whose name matches the argument
pattern. The option can be used multiple times.
Among other use cases, this is useful where a database name is visible
but the database is not dumpable by the user. Examples of this occur in
some managed Postgres services.
I will add this to the September CF.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
pg_dumpall-exclude-database.patchtext/x-patch; name=pg_dumpall-exclude-database.patchDownload
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index 94d76c3..41a7ed7 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -300,6 +300,28 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term><option>--exclude-database=<replaceable class="parameter">dbname</replaceable></option></term>
+ <listitem>
+ <para>
+ Do not dump databases whose name matches
+ <replaceable class="parameter">dbname</replaceable>.
+ Multiple databases can be excluded by writing multiple
+ <option>--exclude-database</option> switches. Also, the
+ <replaceable class="parameter">dbname</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" endterm="app-psql-patterns-title"/>),
+ so multiple databases can also be excluded 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>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>--if-exists</option></term>
<listitem>
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index eb29d31..43058e9 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -52,6 +52,9 @@ static PGconn *connectDatabase(const char *dbname, const char *connstr, const ch
static char *constructConnStr(const char **keywords, const char **values);
static PGresult *executeQuery(PGconn *conn, const char *query);
static void executeCommand(PGconn *conn, const char *query);
+static bool database_excluded(char * datname);
+static void expand_dbname_patterns(PGconn *conn, SimpleStringList *patterns,
+ SimpleStringList *names);
static char pg_dump_bin[MAXPGPATH];
static const char *progname;
@@ -87,6 +90,9 @@ static char role_catalog[10];
static FILE *OPF;
static char *filename = NULL;
+static SimpleStringList database_exclude_patterns = {NULL, NULL};
+static SimpleStringList database_exclude_names = {NULL, NULL};
+
#define exit_nicely(code) exit(code)
int
@@ -123,6 +129,7 @@ main(int argc, char *argv[])
{"column-inserts", no_argument, &column_inserts, 1},
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"exclude-database",required_argument, NULL, 5},
{"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
@@ -318,6 +325,10 @@ main(int argc, char *argv[])
appendPQExpBufferStr(pgdumpopts, " --no-sync");
break;
+ case 5:
+ simple_string_list_append(&database_exclude_patterns, optarg);
+ break;
+
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit_nicely(1);
@@ -449,6 +460,12 @@ main(int argc, char *argv[])
}
/*
+ * Get a list of database names that match the exclude patterns
+ */
+ expand_dbname_patterns(conn, &database_exclude_patterns,
+ &database_exclude_names);
+
+ /*
* Open the output file if required, otherwise use stdout
*/
if (filename)
@@ -614,6 +631,7 @@ help(void)
printf(_(" --column-inserts dump data as INSERT commands with column names\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --exclude-database=PATTERN exclude databases whose name matches PATTERN\n"));
printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --load-via-partition-root load partitions via the root table\n"));
@@ -1351,6 +1369,66 @@ dumpUserConfig(PGconn *conn, const char *username)
destroyPQExpBuffer(buf);
}
+/*
+ * Find a list of database names that match the given patterns.
+ * This is similar to code in pg_dump.c for handling matching table names.
+ */
+static void
+expand_dbname_patterns(PGconn *conn,
+ SimpleStringList *patterns,
+ SimpleStringList *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. It might sometimes result in
+ * duplicate entries in the names list, but we don't care, since a
+ * database is going to be excluded if it matches any entry on the list.
+ */
+
+ for (cell = patterns->head; cell; cell = cell->next)
+ {
+ appendPQExpBuffer(query,
+ "SELECT datname FROM pg_catalog.pg_database n\n");
+ processSQLNamePattern(conn, query, cell->val, false,
+ false, NULL, "datname", NULL, NULL);
+
+ res = executeQuery(conn, query->data);
+ for (i = 0; i < PQntuples(res); i++)
+ {
+ simple_string_list_append(names, PQgetvalue(res, i, 0));
+ }
+
+ PQclear(res);
+ resetPQExpBuffer(query);
+ }
+
+ destroyPQExpBuffer(query);
+}
+
+/*
+ * Is the datname on the list of excluded databases?
+ */
+static bool
+database_excluded(char * datname)
+{
+ SimpleStringListCell *cell;
+
+ for (cell = (&database_exclude_names)->head; cell; cell = cell->next)
+ {
+ if (strcmp(datname, cell->val) == 0)
+ return true;
+ }
+ return false;
+}
/*
* Dump contents of databases.
@@ -1388,6 +1466,15 @@ dumpDatabases(PGconn *conn)
if (strcmp(dbname, "template0") == 0)
continue;
+ /* Skip any explicitly excluded database */
+ if (database_excluded(dbname))
+ {
+ if (verbose)
+ fprintf(stderr, _("%s: excluding database \"%s\"...\n"),
+ progname, dbname);
+ continue;
+ }
+
if (verbose)
fprintf(stderr, _("%s: dumping database \"%s\"...\n"), progname, dbname);
diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index 17edf44..65c6f9f 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -4,7 +4,7 @@ use warnings;
use Config;
use PostgresNode;
use TestLib;
-use Test::More tests => 70;
+use Test::More tests => 72;
my $tempdir = TestLib::tempdir;
my $tempdir_short = TestLib::tempdir_short;
@@ -150,3 +150,8 @@ command_fails_like(
[ 'pg_dumpall', '--if-exists' ],
qr/\Qpg_dumpall: option --if-exists requires option -c\/--clean\E/,
'pg_dumpall: option --if-exists requires option -c/--clean');
+
+command_fails_like(
+ [ 'pg_dumpall', '--exclude-database' ],
+ qr/\Qpg_dumpall: option '--exclude-database' requires an argument\E/,
+ "pg_dumpall: option '--exclude-database' requires an argument");
Among other use cases, this is useful where a database name is visible but
the database is not dumpable by the user. Examples of this occur in some
managed Postgres services.
This looks like a reasonable feature.
I will add this to the September CF.
My 0.02€:
Patch applies cleanly, compiles, and works for me.
A question: would it makes sense to have a symmetrical
--include-database=PATTERN option as well?
Somehow the option does not make much sense when under -g/-r/-t... maybe
it should complain, like it does when the others are used together?
ISTM that it would have been better to issue just one query with an OR
list, but that would require to extend "processSQLNamePattern" a little
bit. Not sure whether it is worth it.
Function "database_excluded": I'd suggest to consider reusing the
"simple_string_list_member" function instead of reimplementing it in a
special case.
XML doc: "--exclude-database=dbname", ISTM that
"--exclude-database=pattern" would be closer to what it is? "Multiple
database can be matched by writing multiple switches". Sure, but it can
also be done with a pattern. The documentation seems to assume that the
argument is one database name, and then changes this afterwards. I'd
suggest to start by saying that a pattern like psql is expected, and then
proceed to simply tell that the option can be repeated, instead of
implying that it is a dbname and then telling that it is a pattern.
The simple list is not freed. Ok, it seems to be part of the design of the
data structure.
--
Fabien.
On Fri, Aug 03, 2018 at 11:08:57PM +0200, Fabien COELHO wrote:
Patch applies cleanly, compiles, and works for me.
Last review has not been addressed, so please note that this has been
marked as returned with feedback.
--
Michael
On 08/03/2018 05:08 PM, Fabien COELHO wrote:
Among other use cases, this is useful where a database name is
visible but the database is not dumpable by the user. Examples of
this occur in some managed Postgres services.This looks like a reasonable feature.
Thanks for the review.
I will add this to the September CF.
My 0.02€:
Patch applies cleanly, compiles, and works for me.
A question: would it makes sense to have a symmetrical
--include-database=PATTERN option as well?
I don't think so. If you only want a few databases, just use pg_dump.
The premise of pg_dumpall is that you want all of them and this switch
provides for exceptions to that.
Somehow the option does not make much sense when under -g/-r/-t...
maybe it should complain, like it does when the others are used together?
Added an error check.
ISTM that it would have been better to issue just one query with an OR
list, but that would require to extend "processSQLNamePattern" a
little bit. Not sure whether it is worth it.
I don't think it is. This uses the same pattern that is used in
pg_dump.c for similar switches.
Function "database_excluded": I'd suggest to consider reusing the
"simple_string_list_member" function instead of reimplementing it in a
special case.
done.
XML doc: "--exclude-database=dbname", ISTM that
"--exclude-database=pattern" would be closer to what it is? "Multiple
database can be matched by writing multiple switches". Sure, but it
can also be done with a pattern. The documentation seems to assume
that the argument is one database name, and then changes this
afterwards. I'd suggest to start by saying that a pattern like psql is
expected, and then proceed to simply tell that the option can be
repeated, instead of implying that it is a dbname and then telling
that it is a pattern.
docco revised.
The simple list is not freed. Ok, it seems to be part of the design of
the data structure.
I don't see much point in freeing it.
revised patch attached.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
pg_dumpall-exclude-v3.patchtext/x-patch; name=pg_dumpall-exclude-v3.patchDownload
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index c51a130..f0e1697 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -300,6 +300,28 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term><option>--exclude-database=<replaceable class="parameter">pattern</replaceable></option></term>
+ <listitem>
+ <para>
+ Do not dump databases whose name matches
+ <replaceable class="parameter">pattern</replaceable>.
+ Multiple patterns can be excluded by writing multiple
+ <option>--exclude-database</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" endterm="app-psql-patterns-title"/>),
+ so multiple databases can also be excluded 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>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>--if-exists</option></term>
<listitem>
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index eb29d31..2bf8743 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -52,6 +52,8 @@ static PGconn *connectDatabase(const char *dbname, const char *connstr, const ch
static char *constructConnStr(const char **keywords, const char **values);
static PGresult *executeQuery(PGconn *conn, const char *query);
static void executeCommand(PGconn *conn, const char *query);
+static void expand_dbname_patterns(PGconn *conn, SimpleStringList *patterns,
+ SimpleStringList *names);
static char pg_dump_bin[MAXPGPATH];
static const char *progname;
@@ -87,6 +89,9 @@ static char role_catalog[10];
static FILE *OPF;
static char *filename = NULL;
+static SimpleStringList database_exclude_patterns = {NULL, NULL};
+static SimpleStringList database_exclude_names = {NULL, NULL};
+
#define exit_nicely(code) exit(code)
int
@@ -123,6 +128,7 @@ main(int argc, char *argv[])
{"column-inserts", no_argument, &column_inserts, 1},
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"exclude-database",required_argument, NULL, 5},
{"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
@@ -318,6 +324,10 @@ main(int argc, char *argv[])
appendPQExpBufferStr(pgdumpopts, " --no-sync");
break;
+ case 5:
+ simple_string_list_append(&database_exclude_patterns, optarg);
+ break;
+
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit_nicely(1);
@@ -334,6 +344,16 @@ main(int argc, char *argv[])
exit_nicely(1);
}
+ if (database_exclude_patterns.head != NULL &&
+ (globals_only || roles_only || tablespaces_only))
+ {
+ fprintf(stderr, _("%s: option --exclude-database cannot be used together with -g/--globals-only, -r/--roles-only or -t/--tablespaces-only\n"),
+ progname);
+ fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+ progname);
+ exit_nicely(1);
+ }
+
/* Make sure the user hasn't specified a mix of globals-only options */
if (globals_only && roles_only)
{
@@ -449,6 +469,12 @@ main(int argc, char *argv[])
}
/*
+ * Get a list of database names that match the exclude patterns
+ */
+ expand_dbname_patterns(conn, &database_exclude_patterns,
+ &database_exclude_names);
+
+ /*
* Open the output file if required, otherwise use stdout
*/
if (filename)
@@ -614,6 +640,7 @@ help(void)
printf(_(" --column-inserts dump data as INSERT commands with column names\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --exclude-database=PATTERN exclude databases whose name matches PATTERN\n"));
printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --load-via-partition-root load partitions via the root table\n"));
@@ -1351,6 +1378,49 @@ dumpUserConfig(PGconn *conn, const char *username)
destroyPQExpBuffer(buf);
}
+/*
+ * Find a list of database names that match the given patterns.
+ * This is similar to code in pg_dump.c for handling matching table names.
+ */
+static void
+expand_dbname_patterns(PGconn *conn,
+ SimpleStringList *patterns,
+ SimpleStringList *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)
+ {
+ appendPQExpBuffer(query,
+ "SELECT datname FROM pg_catalog.pg_database n\n");
+ processSQLNamePattern(conn, query, cell->val, false,
+ false, NULL, "datname", NULL, NULL);
+
+ res = executeQuery(conn, query->data);
+ for (i = 0; i < PQntuples(res); i++)
+ {
+ simple_string_list_append(names, PQgetvalue(res, i, 0));
+ }
+
+ PQclear(res);
+ resetPQExpBuffer(query);
+ }
+
+ destroyPQExpBuffer(query);
+}
/*
* Dump contents of databases.
@@ -1388,6 +1458,15 @@ dumpDatabases(PGconn *conn)
if (strcmp(dbname, "template0") == 0)
continue;
+ /* Skip any explicitly excluded database */
+ if (simple_string_list_member(&database_exclude_names, dbname))
+ {
+ if (verbose)
+ fprintf(stderr, _("%s: excluding database \"%s\"...\n"),
+ progname, dbname);
+ continue;
+ }
+
if (verbose)
fprintf(stderr, _("%s: dumping database \"%s\"...\n"), progname, dbname);
diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index 17edf44..34dd893 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -4,7 +4,7 @@ use warnings;
use Config;
use PostgresNode;
use TestLib;
-use Test::More tests => 70;
+use Test::More tests => 74;
my $tempdir = TestLib::tempdir;
my $tempdir_short = TestLib::tempdir_short;
@@ -150,3 +150,14 @@ command_fails_like(
[ 'pg_dumpall', '--if-exists' ],
qr/\Qpg_dumpall: option --if-exists requires option -c\/--clean\E/,
'pg_dumpall: option --if-exists requires option -c/--clean');
+
+command_fails_like(
+ [ 'pg_dumpall', '--exclude-database' ],
+ qr/\Qpg_dumpall: option '--exclude-database' requires an argument\E/,
+ 'pg_dumpall: option --exclude-database requires an argument');
+
+# also fails for -r and -t, but it seems pointless to add more tests for those.
+command_fails_like(
+ [ 'pg_dumpall', '--exclude-database=foo', '--globals-only' ],
+ qr/\Qpg_dumpall: option --exclude-database cannot be used together with -g\/--globals-only\E/,
+ 'pg_dumpall: option --exclude-database cannot be used together with -g/--globals-only');
Hello Andrew,
A question: would it makes sense to have a symmetrical
--include-database=PATTERN option as well?I don't think so. If you only want a few databases, just use pg_dump. The
premise of pg_dumpall is that you want all of them and this switch provides
for exceptions to that.
Ok, sounds reasonable.
Somehow the option does not make much sense when under -g/-r/-t... maybe it
should complain, like it does when the others are used together?Added an error check.
Ok.
ISTM that it would have been better to issue just one query with an OR
list, but that would require to extend "processSQLNamePattern" a little
bit. Not sure whether it is worth it.I don't think it is. This uses the same pattern that is used in pg_dump.c for
similar switches.
Ok.
revised patch attached.
Patch applies cleanly, compiles, make check ok, pg_dump tap tests ok, doc
build ok.
Very minor comments:
Missing space after comma:
+ {"exclude-database",required_argument, NULL, 5},
Now that C99 is okay, ISTM that both for loops in expand_dbname_patterns
could benefit from using loop-local variables:
for (SimpleStringListCell *cell = ...
for (int i = ...
About the documentation:
"When using wildcards, be careful to quote the pattern if needed to prevent
the shell from expanding the wildcards."
I'd suggest to consider simplifying the end, maybe "to prevent shell
wildcard expansion".
The feature is not tested per se. Maybe one existing tap test could be
extended with minimal fuss to use it, eg --exclude-database='[a-z]*'
should be close to only keeping the global stuff? I noticed an "exclude
table" test already exists.
--
Fabien.
On 10/13/2018 10:07 AM, Fabien COELHO wrote:
Hello Andrew,
A question: would it makes sense to have a symmetrical
--include-database=PATTERN option as well?I don't think so. If you only want a few databases, just use pg_dump.
The premise of pg_dumpall is that you want all of them and this
switch provides for exceptions to that.Ok, sounds reasonable.
Somehow the option does not make much sense when under -g/-r/-t...
maybe it should complain, like it does when the others are used
together?Added an error check.
Ok.
ISTM that it would have been better to issue just one query with an
OR list, but that would require to extend "processSQLNamePattern" a
little bit. Not sure whether it is worth it.I don't think it is. This uses the same pattern that is used in
pg_dump.c for similar switches.Ok.
revised patch attached.
Patch applies cleanly, compiles, make check ok, pg_dump tap tests ok,
doc build ok.Very minor comments:
Missing space after comma:
+ {"exclude-database",required_argument, NULL, 5},
Now that C99 is okay, ISTM that both for loops in
expand_dbname_patterns could benefit from using loop-local variables:for (SimpleStringListCell *cell = ...
for (int i = ...About the documentation:
"When using wildcards, be careful to quote the pattern if needed to
prevent
the shell from expanding the wildcards."I'd suggest to consider simplifying the end, maybe "to prevent shell
wildcard expansion".The feature is not tested per se. Maybe one existing tap test could be
extended with minimal fuss to use it, eg --exclude-database='[a-z]*'
should be close to only keeping the global stuff? I noticed an
"exclude table" test already exists.
This patch addresses all these concerns.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
pg_dumpall-exclude-v4.patchtext/x-patch; name=pg_dumpall-exclude-v4.patchDownload
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index c51a130..973b995 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -300,6 +300,27 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term><option>--exclude-database=<replaceable class="parameter">pattern</replaceable></option></term>
+ <listitem>
+ <para>
+ Do not dump databases whose name matches
+ <replaceable class="parameter">pattern</replaceable>.
+ Multiple patterns can be excluded by writing multiple
+ <option>--exclude-database</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" endterm="app-psql-patterns-title"/>),
+ so multiple databases can also be excluded by writing wildcard
+ characters in the pattern. When using wildcards, be careful to
+ quote the pattern if needed to prevent shell wildcard expansion.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>--if-exists</option></term>
<listitem>
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 5176626..ee7bc8e 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -52,6 +52,8 @@ static PGconn *connectDatabase(const char *dbname, const char *connstr, const ch
static char *constructConnStr(const char **keywords, const char **values);
static PGresult *executeQuery(PGconn *conn, const char *query);
static void executeCommand(PGconn *conn, const char *query);
+static void expand_dbname_patterns(PGconn *conn, SimpleStringList *patterns,
+ SimpleStringList *names);
static char pg_dump_bin[MAXPGPATH];
static const char *progname;
@@ -87,6 +89,9 @@ static char role_catalog[10];
static FILE *OPF;
static char *filename = NULL;
+static SimpleStringList database_exclude_patterns = {NULL, NULL};
+static SimpleStringList database_exclude_names = {NULL, NULL};
+
#define exit_nicely(code) exit(code)
int
@@ -123,6 +128,7 @@ main(int argc, char *argv[])
{"column-inserts", no_argument, &column_inserts, 1},
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"exclude-database", required_argument, NULL, 5},
{"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
@@ -318,6 +324,10 @@ main(int argc, char *argv[])
appendPQExpBufferStr(pgdumpopts, " --no-sync");
break;
+ case 5:
+ simple_string_list_append(&database_exclude_patterns, optarg);
+ break;
+
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit_nicely(1);
@@ -334,6 +344,16 @@ main(int argc, char *argv[])
exit_nicely(1);
}
+ if (database_exclude_patterns.head != NULL &&
+ (globals_only || roles_only || tablespaces_only))
+ {
+ fprintf(stderr, _("%s: option --exclude-database cannot be used together with -g/--globals-only, -r/--roles-only or -t/--tablespaces-only\n"),
+ progname);
+ fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+ progname);
+ exit_nicely(1);
+ }
+
/* Make sure the user hasn't specified a mix of globals-only options */
if (globals_only && roles_only)
{
@@ -449,6 +469,12 @@ main(int argc, char *argv[])
}
/*
+ * Get a list of database names that match the exclude patterns
+ */
+ expand_dbname_patterns(conn, &database_exclude_patterns,
+ &database_exclude_names);
+
+ /*
* Open the output file if required, otherwise use stdout
*/
if (filename)
@@ -614,6 +640,7 @@ help(void)
printf(_(" --column-inserts dump data as INSERT commands with column names\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --exclude-database=PATTERN exclude databases whose name matches PATTERN\n"));
printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --load-via-partition-root load partitions via the root table\n"));
@@ -1351,6 +1378,47 @@ dumpUserConfig(PGconn *conn, const char *username)
destroyPQExpBuffer(buf);
}
+/*
+ * Find a list of database names that match the given patterns.
+ * This is similar to code in pg_dump.c for handling matching table names.
+ */
+static void
+expand_dbname_patterns(PGconn *conn,
+ SimpleStringList *patterns,
+ SimpleStringList *names)
+{
+ PQExpBuffer query;
+ PGresult *res;
+
+ 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 (SimpleStringListCell *cell = patterns->head; cell; cell = cell->next)
+ {
+ appendPQExpBuffer(query,
+ "SELECT datname FROM pg_catalog.pg_database n\n");
+ processSQLNamePattern(conn, query, cell->val, false,
+ false, NULL, "datname", NULL, NULL);
+
+ res = executeQuery(conn, query->data);
+ for (int i = 0; i < PQntuples(res); i++)
+ {
+ simple_string_list_append(names, PQgetvalue(res, i, 0));
+ }
+
+ PQclear(res);
+ resetPQExpBuffer(query);
+ }
+
+ destroyPQExpBuffer(query);
+}
/*
* Dump contents of databases.
@@ -1388,6 +1456,15 @@ dumpDatabases(PGconn *conn)
if (strcmp(dbname, "template0") == 0)
continue;
+ /* Skip any explicitly excluded database */
+ if (simple_string_list_member(&database_exclude_names, dbname))
+ {
+ if (verbose)
+ fprintf(stderr, _("%s: excluding database \"%s\"...\n"),
+ progname, dbname);
+ continue;
+ }
+
if (verbose)
fprintf(stderr, _("%s: dumping database \"%s\"...\n"), progname, dbname);
diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index de00dbc..67c4080 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -4,7 +4,7 @@ use warnings;
use Config;
use PostgresNode;
use TestLib;
-use Test::More tests => 72;
+use Test::More tests => 76;
my $tempdir = TestLib::tempdir;
my $tempdir_short = TestLib::tempdir_short;
@@ -156,3 +156,14 @@ command_fails_like(
qr/\Qpg_restore: options -C\/--create and -1\/--single-transaction cannot be used together\E/,
'pg_restore: options -C\/--create and -1\/--single-transaction cannot be used together'
);
+
+command_fails_like(
+ [ 'pg_dumpall', '--exclude-database' ],
+ qr/\Qpg_dumpall: option '--exclude-database' requires an argument\E/,
+ 'pg_dumpall: option --exclude-database requires an argument');
+
+# also fails for -r and -t, but it seems pointless to add more tests for those.
+command_fails_like(
+ [ 'pg_dumpall', '--exclude-database=foo', '--globals-only' ],
+ qr/\Qpg_dumpall: option --exclude-database cannot be used together with -g\/--globals-only\E/,
+ 'pg_dumpall: option --exclude-database cannot be used together with -g/--globals-only');
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index ec751a7..37d804b 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -224,6 +224,12 @@ my %pgdump_runs = (
"--file=$tempdir/pg_dumpall_dbprivs.sql",
],
},
+ pg_dumpall_exclude => {
+ dump_cmd => [
+ 'pg_dumpall', '-v', "--file=$tempdir/pg_dumpall_exclude.sql",
+ '--exclude-database', '*dump*', '--no-sync',
+ ],
+ },
no_blobs => {
dump_cmd => [
'pg_dump', '--no-sync',
@@ -387,6 +393,7 @@ my %full_runs = (
no_owner => 1,
no_privs => 1,
pg_dumpall_dbprivs => 1,
+ pg_dumpall_exclude => 1,
schema_only => 1,
with_oids => 1,);
@@ -452,6 +459,7 @@ my %tests = (
pg_dumpall_dbprivs => 1,
pg_dumpall_globals => 1,
pg_dumpall_globals_clean => 1,
+ pg_dumpall_exclude => 1,
},
},
@@ -1386,6 +1394,7 @@ my %tests = (
regexp => qr/^CREATE ROLE regress_dump_test_role;/m,
like => {
pg_dumpall_dbprivs => 1,
+ pg_dumpall_exclude => 1,
pg_dumpall_globals => 1,
pg_dumpall_globals_clean => 1,
},
@@ -2515,6 +2524,7 @@ my %tests = (
no_owner => 1,
only_dump_test_schema => 1,
pg_dumpall_dbprivs => 1,
+ pg_dumpall_exclude => 1,
schema_only => 1,
section_post_data => 1,
test_schema_plus_blobs => 1,
@@ -2586,6 +2596,7 @@ my %tests = (
no_privs => 1,
no_owner => 1,
pg_dumpall_dbprivs => 1,
+ pg_dumpall_exclude => 1,
role => 1,
schema_only => 1,
section_post_data => 1,
Hello Andrew,
This patch addresses all these concerns.
Patch v4 applies cleanly, compiles, doc generation ok, global & local
tests ok.
Tiny comments: there is a useless added blank line at the beginning of the
added varlistenry.
I have recreated the CF entry and put the patch to ready... but I've must
have mixed up something because now there are two entries:-(
Could anywone remove the duplicate entry (1859 & 1860 are the same)?
Peter??
--
Fabien.
On 10/31/2018 12:44 PM, Fabien COELHO wrote:
Hello Andrew,
This patch addresses all these concerns.
Patch v4 applies cleanly, compiles, doc generation ok, global & local
tests ok.Tiny comments: there is a useless added blank line at the beginning of
the added varlistenry.I have recreated the CF entry and put the patch to ready... but I've
must have mixed up something because now there are two entries:-(Could anywone remove the duplicate entry (1859 & 1860 are the same)?
Peter??
:-( My fault, I just created a new one.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
:-( My fault, I just created a new one.
Hmmm... so did I:-) We did it a few minutes apart. I did not find yours
when I first searched, then I proceeded to try to move the previous CF
entry which had been marked as "returned" but this was rejected, so I
recreated the one without checking whether it had appeared in between.
Hopefully someone can remove it…
--
Fabien.
On Wed, Oct 31, 2018 at 05:44:26PM +0100, Fabien COELHO wrote:
Patch v4 applies cleanly, compiles, doc generation ok, global & local tests
ok.
+# also fails for -r and -t, but it seems pointless to add more tests
for those.
+command_fails_like(
+ [ 'pg_dumpall', '--exclude-database=foo', '--globals-only' ],
+ qr/\Qpg_dumpall: option --exclude-database cannot be used
together with -g\/--globals-only\E/,
+ 'pg_dumpall: option --exclude-database cannot be used together
with -g/--globals-only');Usually testing all combinations is preferred, as well as having one
error message for each pattern, which is also more consistent with all
the other sanity checks in pg_dumpall.c and such.
--
Michael
The comment in expand_dbname_patterns is ungrammatical and mentions
"OID" rather than "name", so I suggest
/*
* The loop below might sometimes result in duplicate entries in the
* output name list, but we don't care.
*/
I'm not sure this is grammatical either:
exclude databases whose name matches PATTERN
I would have written it like this:
exclude databases whose names match PATTERN
but I'm not sure (each database has only one name, of course, but aren't
you talking about multiple databases there?)
Other than that, the patch seems fine to me -- I tested and it works as
intended.
Personally I would say "See also expand_table_name_patterns" instead of
"This is similar to code in pg_dump.c for handling matching table names.",
as well as mention this function in expand_table_name_patterns' comment.
(No need to mention expand_schema_name_patterns, since these are
adjacent.) But this is mostly stylistic and left to your own judgement.
In the long run, I think we should add an option to processSQLNamePattern
to use OR instead of AND, which would fix both this problem as well as
pg_dump's. I don't think that's important enough to stall this patch.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 11/17/18 9:55 AM, Alvaro Herrera wrote:
The comment in expand_dbname_patterns is ungrammatical and mentions
"OID" rather than "name", so I suggest/*
* The loop below might sometimes result in duplicate entries in the
* output name list, but we don't care.
*/
Will fix.
I'm not sure this is grammatical either:
exclude databases whose name matches PATTERN
I would have written it like this:
exclude databases whose names match PATTERN
but I'm not sure (each database has only one name, of course, but aren't
you talking about multiple databases there?)
I think the original is grammatical.
Other than that, the patch seems fine to me -- I tested and it works as
intended.Personally I would say "See also expand_table_name_patterns" instead of
"This is similar to code in pg_dump.c for handling matching table names.",
as well as mention this function in expand_table_name_patterns' comment.
(No need to mention expand_schema_name_patterns, since these are
adjacent.) But this is mostly stylistic and left to your own judgement.In the long run, I think we should add an option to processSQLNamePattern
to use OR instead of AND, which would fix both this problem as well as
pg_dump's. I don't think that's important enough to stall this patch.
Thanks for the review.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Sun, Nov 18, 2018 at 7:41 PM Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote:
On 11/17/18 9:55 AM, Alvaro Herrera wrote:
The comment in expand_dbname_patterns is ungrammatical and mentions
"OID" rather than "name", so I suggestWill fix.
Other than that, the patch seems fine to me -- I tested and it works as
intended.Personally I would say "See also expand_table_name_patterns" instead of
"This is similar to code in pg_dump.c for handling matching table names.",
as well as mention this function in expand_table_name_patterns' comment.
(No need to mention expand_schema_name_patterns, since these are
adjacent.) But this is mostly stylistic and left to your own judgement.In the long run, I think we should add an option to processSQLNamePattern
to use OR instead of AND, which would fix both this problem as well as
pg_dump's. I don't think that's important enough to stall this patch.Thanks for the review.
Unfortunately judging from cfbot output patch needs to be rebased, could you
please post an updated version with those fixes mentioned above?
On 11/18/18 1:41 PM, Andrew Dunstan wrote:
On 11/17/18 9:55 AM, Alvaro Herrera wrote:
The comment in expand_dbname_patterns is ungrammatical and mentions
"OID" rather than "name", so I suggest/*
* The loop below might sometimes result in duplicate entries in the
* output name list, but we don't care.
*/Will fix.
I'm not sure this is grammatical either:
exclude databases whose name matches PATTERN
I would have written it like this:
exclude databases whose names match PATTERN
but I'm not sure (each database has only one name, of course, but aren't
you talking about multiple databases there?)I think the original is grammatical.
Other than that, the patch seems fine to me -- I tested and it works as
intended.Personally I would say "See also expand_table_name_patterns" instead of
"This is similar to code in pg_dump.c for handling matching table
names.",
as well as mention this function in expand_table_name_patterns' comment.
(No need to mention expand_schema_name_patterns, since these are
adjacent.) But this is mostly stylistic and left to your own judgement.In the long run, I think we should add an option to
processSQLNamePattern
to use OR instead of AND, which would fix both this problem as well as
pg_dump's. I don't think that's important enough to stall this patch.Thanks for the review.
Rebased and updated patch attached.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
pg_dumpall-exclude-v5.patchtext/x-patch; name=pg_dumpall-exclude-v5.patchDownload
diff --git a/doc/src/sgml/ref/pg_dumpall.sgml b/doc/src/sgml/ref/pg_dumpall.sgml
index c51a130f43..973b9955bf 100644
--- a/doc/src/sgml/ref/pg_dumpall.sgml
+++ b/doc/src/sgml/ref/pg_dumpall.sgml
@@ -300,6 +300,27 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term><option>--exclude-database=<replaceable class="parameter">pattern</replaceable></option></term>
+ <listitem>
+ <para>
+ Do not dump databases whose name matches
+ <replaceable class="parameter">pattern</replaceable>.
+ Multiple patterns can be excluded by writing multiple
+ <option>--exclude-database</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" endterm="app-psql-patterns-title"/>),
+ so multiple databases can also be excluded by writing wildcard
+ characters in the pattern. When using wildcards, be careful to
+ quote the pattern if needed to prevent shell wildcard expansion.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>--if-exists</option></term>
<listitem>
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index d583154fba..3aa0f5f1fa 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1276,7 +1276,8 @@ expand_schema_name_patterns(Archive *fout,
/*
* Find the OIDs of all tables matching the given list of patterns,
- * and append them to the given OID list.
+ * and append them to the given OID list. See also expand_dbname_patterns()
+ * in pg_dumpall.c
*/
static void
expand_table_name_patterns(Archive *fout,
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 5176626476..f57ff2fe3f 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -52,6 +52,8 @@ static PGconn *connectDatabase(const char *dbname, const char *connstr, const ch
static char *constructConnStr(const char **keywords, const char **values);
static PGresult *executeQuery(PGconn *conn, const char *query);
static void executeCommand(PGconn *conn, const char *query);
+static void expand_dbname_patterns(PGconn *conn, SimpleStringList *patterns,
+ SimpleStringList *names);
static char pg_dump_bin[MAXPGPATH];
static const char *progname;
@@ -87,6 +89,9 @@ static char role_catalog[10];
static FILE *OPF;
static char *filename = NULL;
+static SimpleStringList database_exclude_patterns = {NULL, NULL};
+static SimpleStringList database_exclude_names = {NULL, NULL};
+
#define exit_nicely(code) exit(code)
int
@@ -123,6 +128,7 @@ main(int argc, char *argv[])
{"column-inserts", no_argument, &column_inserts, 1},
{"disable-dollar-quoting", no_argument, &disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
+ {"exclude-database", required_argument, NULL, 5},
{"if-exists", no_argument, &if_exists, 1},
{"inserts", no_argument, &inserts, 1},
{"lock-wait-timeout", required_argument, NULL, 2},
@@ -318,6 +324,10 @@ main(int argc, char *argv[])
appendPQExpBufferStr(pgdumpopts, " --no-sync");
break;
+ case 5:
+ simple_string_list_append(&database_exclude_patterns, optarg);
+ break;
+
default:
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit_nicely(1);
@@ -334,6 +344,16 @@ main(int argc, char *argv[])
exit_nicely(1);
}
+ if (database_exclude_patterns.head != NULL &&
+ (globals_only || roles_only || tablespaces_only))
+ {
+ fprintf(stderr, _("%s: option --exclude-database cannot be used together with -g/--globals-only, -r/--roles-only or -t/--tablespaces-only\n"),
+ progname);
+ fprintf(stderr, _("Try \"%s --help\" for more information.\n"),
+ progname);
+ exit_nicely(1);
+ }
+
/* Make sure the user hasn't specified a mix of globals-only options */
if (globals_only && roles_only)
{
@@ -448,6 +468,12 @@ main(int argc, char *argv[])
}
}
+ /*
+ * Get a list of database names that match the exclude patterns
+ */
+ expand_dbname_patterns(conn, &database_exclude_patterns,
+ &database_exclude_names);
+
/*
* Open the output file if required, otherwise use stdout
*/
@@ -614,6 +640,7 @@ help(void)
printf(_(" --column-inserts dump data as INSERT commands with column names\n"));
printf(_(" --disable-dollar-quoting disable dollar quoting, use SQL standard quoting\n"));
printf(_(" --disable-triggers disable triggers during data-only restore\n"));
+ printf(_(" --exclude-database=PATTERN exclude databases whose name matches PATTERN\n"));
printf(_(" --if-exists use IF EXISTS when dropping objects\n"));
printf(_(" --inserts dump data as INSERT commands, rather than COPY\n"));
printf(_(" --load-via-partition-root load partitions via the root table\n"));
@@ -1351,6 +1378,48 @@ dumpUserConfig(PGconn *conn, const char *username)
destroyPQExpBuffer(buf);
}
+/*
+ * Find a list of database names that match the given patterns.
+ * See also expand_table_name_patterns() in pg_dump.c
+ */
+static void
+expand_dbname_patterns(PGconn *conn,
+ SimpleStringList *patterns,
+ SimpleStringList *names)
+{
+ PQExpBuffer query;
+ PGresult *res;
+
+ if (patterns->head == NULL)
+ return; /* nothing to do */
+
+ query = createPQExpBuffer();
+
+ /*
+ * The loop below runs multiple SELECTs, which might sometimes result in
+ * duplicate entries in the name list, but we don't care, since all
+ * we're going to do is test membership of the list.
+ */
+
+ for (SimpleStringListCell *cell = patterns->head; cell; cell = cell->next)
+ {
+ appendPQExpBuffer(query,
+ "SELECT datname FROM pg_catalog.pg_database n\n");
+ processSQLNamePattern(conn, query, cell->val, false,
+ false, NULL, "datname", NULL, NULL);
+
+ res = executeQuery(conn, query->data);
+ for (int i = 0; i < PQntuples(res); i++)
+ {
+ simple_string_list_append(names, PQgetvalue(res, i, 0));
+ }
+
+ PQclear(res);
+ resetPQExpBuffer(query);
+ }
+
+ destroyPQExpBuffer(query);
+}
/*
* Dump contents of databases.
@@ -1388,6 +1457,15 @@ dumpDatabases(PGconn *conn)
if (strcmp(dbname, "template0") == 0)
continue;
+ /* Skip any explicitly excluded database */
+ if (simple_string_list_member(&database_exclude_names, dbname))
+ {
+ if (verbose)
+ fprintf(stderr, _("%s: excluding database \"%s\"...\n"),
+ progname, dbname);
+ continue;
+ }
+
if (verbose)
fprintf(stderr, _("%s: dumping database \"%s\"...\n"), progname, dbname);
diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index a875d540b8..59b77214c6 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -4,7 +4,7 @@ use warnings;
use Config;
use PostgresNode;
use TestLib;
-use Test::More tests => 70;
+use Test::More tests => 74;
my $tempdir = TestLib::tempdir;
my $tempdir_short = TestLib::tempdir_short;
@@ -150,3 +150,14 @@ command_fails_like(
qr/\Qpg_restore: options -C\/--create and -1\/--single-transaction cannot be used together\E/,
'pg_restore: options -C\/--create and -1\/--single-transaction cannot be used together'
);
+
+command_fails_like(
+ [ 'pg_dumpall', '--exclude-database' ],
+ qr/\Qpg_dumpall: option '--exclude-database' requires an argument\E/,
+ 'pg_dumpall: option --exclude-database requires an argument');
+
+# also fails for -r and -t, but it seems pointless to add more tests for those.
+command_fails_like(
+ [ 'pg_dumpall', '--exclude-database=foo', '--globals-only' ],
+ qr/\Qpg_dumpall: option --exclude-database cannot be used together with -g\/--globals-only\E/,
+ 'pg_dumpall: option --exclude-database cannot be used together with -g/--globals-only');
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 2afd950591..5c3a28f8db 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -224,6 +224,12 @@ my %pgdump_runs = (
"--file=$tempdir/pg_dumpall_dbprivs.sql",
],
},
+ pg_dumpall_exclude => {
+ dump_cmd => [
+ 'pg_dumpall', '-v', "--file=$tempdir/pg_dumpall_exclude.sql",
+ '--exclude-database', '*dump*', '--no-sync',
+ ],
+ },
no_blobs => {
dump_cmd => [
'pg_dump', '--no-sync',
@@ -380,6 +386,7 @@ my %full_runs = (
no_owner => 1,
no_privs => 1,
pg_dumpall_dbprivs => 1,
+ pg_dumpall_exclude => 1,
schema_only => 1,);
# This is where the actual tests are defined.
@@ -444,6 +451,7 @@ my %tests = (
pg_dumpall_dbprivs => 1,
pg_dumpall_globals => 1,
pg_dumpall_globals_clean => 1,
+ pg_dumpall_exclude => 1,
},
},
@@ -1318,6 +1326,7 @@ my %tests = (
regexp => qr/^CREATE ROLE regress_dump_test_role;/m,
like => {
pg_dumpall_dbprivs => 1,
+ pg_dumpall_exclude => 1,
pg_dumpall_globals => 1,
pg_dumpall_globals_clean => 1,
},
@@ -2387,6 +2396,7 @@ my %tests = (
no_owner => 1,
only_dump_test_schema => 1,
pg_dumpall_dbprivs => 1,
+ pg_dumpall_exclude => 1,
schema_only => 1,
section_post_data => 1,
test_schema_plus_blobs => 1,
@@ -2457,6 +2467,7 @@ my %tests = (
no_privs => 1,
no_owner => 1,
pg_dumpall_dbprivs => 1,
+ pg_dumpall_exclude => 1,
role => 1,
schema_only => 1,
section_post_data => 1,
On Fri, Nov 30, 2018 at 04:26:41PM -0500, Andrew Dunstan wrote:
On 11/18/18 1:41 PM, Andrew Dunstan wrote:
On 11/17/18 9:55 AM, Alvaro Herrera wrote:
In the long run, I think we should add an option to processSQLNamePattern
to use OR instead of AND, which would fix both this problem as well as
pg_dump's. I don't think that's important enough to stall this patch.
Agreed. This patch is useful in itself. This option would be nice to
have, and this routine interface would begin to grow too many boolean
switches to my taste so I'd rather use some flags instead.
The patch is doing its work, however I have spotted an issue in the
format of the dumps generated. Each time an excluded database is
processed its set of SET queries (from _doSetFixedOutputState) as well
as the header "PostgreSQL database dump" gets generated. I think that
this data should not show up.
--
Michael
On 12/18/18 11:53 PM, Michael Paquier wrote:
On Fri, Nov 30, 2018 at 04:26:41PM -0500, Andrew Dunstan wrote:
On 11/18/18 1:41 PM, Andrew Dunstan wrote:
On 11/17/18 9:55 AM, Alvaro Herrera wrote:
In the long run, I think we should add an option to processSQLNamePattern
to use OR instead of AND, which would fix both this problem as well as
pg_dump's.� I don't think that's important enough to stall this patch.Agreed. This patch is useful in itself. This option would be nice to
have, and this routine interface would begin to grow too many boolean
switches to my taste so I'd rather use some flags instead.The patch is doing its work, however I have spotted an issue in the
format of the dumps generated. Each time an excluded database is
processed its set of SET queries (from _doSetFixedOutputState) as well
as the header "PostgreSQL database dump" gets generated. I think that
this data should not show up.
I'll take a look.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 12/19/18 3:55 PM, Andrew Dunstan wrote:
On 12/18/18 11:53 PM, Michael Paquier wrote:
On Fri, Nov 30, 2018 at 04:26:41PM -0500, Andrew Dunstan wrote:
On 11/18/18 1:41 PM, Andrew Dunstan wrote:
On 11/17/18 9:55 AM, Alvaro Herrera wrote:
In the long run, I think we should add an option to processSQLNamePattern
to use OR instead of AND, which would fix both this problem as well as
pg_dump's.� I don't think that's important enough to stall this patch.Agreed. This patch is useful in itself. This option would be nice to
have, and this routine interface would begin to grow too many boolean
switches to my taste so I'd rather use some flags instead.The patch is doing its work, however I have spotted an issue in the
format of the dumps generated. Each time an excluded database is
processed its set of SET queries (from _doSetFixedOutputState) as well
as the header "PostgreSQL database dump" gets generated. I think that
this data should not show up.I'll take a look.
I think you're mistaken. The following example shows this clearly -
there is nothing corresponding to the 20 excluded databases. What you're
referring to appears to be the statements that preceded the 'CREATE
DATABASE' statement. That's to be excpected.
cheers
andrew
andrew@emma:inst (pg_dumpall--exclude)*$ for x in `seq 1 20` ; do
bin/createdb ex$x; done
andrew@emma:inst (pg_dumpall--exclude)*$ bin/createdb inc
andrew@emma:inst (pg_dumpall--exclude)*$ bin/pg_dumpall
--exclude-database "ex*"
--
-- PostgreSQL database cluster dump
--
SET default_transaction_read_only = off;
SET client_encoding = 'UTF8';
SET standard_conforming_strings = on;
--
-- Roles
--
CREATE ROLE andrew;
ALTER ROLE andrew WITH SUPERUSER INHERIT CREATEROLE CREATEDB LOGIN
REPLICATION BYPASSRLS;
\connect template1
--
-- PostgreSQL database dump
--
-- Dumped from database version 12devel
-- Dumped by pg_dump version 12devel
SET statement_timeout = 0;
SET lock_timeout = 0;
SET idle_in_transaction_session_timeout = 0;
SET client_encoding = 'UTF8';
SET standard_conforming_strings = on;
SELECT pg_catalog.set_config('search_path', '', false);
SET check_function_bodies = false;
SET client_min_messages = warning;
SET row_security = off;
--
-- PostgreSQL database dump complete
--
--
-- PostgreSQL database dump
--
-- Dumped from database version 12devel
-- Dumped by pg_dump version 12devel
SET statement_timeout = 0;
SET lock_timeout = 0;
SET idle_in_transaction_session_timeout = 0;
SET client_encoding = 'UTF8';
SET standard_conforming_strings = on;
SELECT pg_catalog.set_config('search_path', '', false);
SET check_function_bodies = false;
SET client_min_messages = warning;
SET row_security = off;
--
-- Name: inc; Type: DATABASE; Schema: -; Owner: andrew
--
CREATE DATABASE inc WITH TEMPLATE = template0 ENCODING = 'UTF8'
LC_COLLATE = 'en_US.UTF-8' LC_CTYPE = 'en_US.UTF-8';
ALTER DATABASE inc OWNER TO andrew;
\connect inc
SET statement_timeout = 0;
SET lock_timeout = 0;
SET idle_in_transaction_session_timeout = 0;
SET client_encoding = 'UTF8';
SET standard_conforming_strings = on;
SELECT pg_catalog.set_config('search_path', '', false);
SET check_function_bodies = false;
SET client_min_messages = warning;
SET row_security = off;
--
-- PostgreSQL database dump complete
--
\connect postgres
--
-- PostgreSQL database dump
--
-- Dumped from database version 12devel
-- Dumped by pg_dump version 12devel
SET statement_timeout = 0;
SET lock_timeout = 0;
SET idle_in_transaction_session_timeout = 0;
SET client_encoding = 'UTF8';
SET standard_conforming_strings = on;
SELECT pg_catalog.set_config('search_path', '', false);
SET check_function_bodies = false;
SET client_min_messages = warning;
SET row_security = off;
--
-- PostgreSQL database dump complete
--
--
-- PostgreSQL database cluster dump complete
--
�
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Mon, Dec 24, 2018 at 02:46:48PM -0500, Andrew Dunstan wrote:
I think you're mistaken. The following example shows this clearly -
there is nothing corresponding to the 20 excluded databases. What you're
referring to appears to be the statements that preceded the 'CREATE
DATABASE' statement. That's to be excpected.
I would have believed that excluding a database removes a full section
from "PostgreSQL database dump" to "PostgreSQL database dump
complete", and not only a portion of it. Sorry for sounding
nit-picky.
--
Michael
Hello Andrew,
Rebased and updated patch attached.
Here is a review of v5, sorry for the delay.
Patch applies cleanly, compiles, "make check" is ok.
I do not see Michaël's issue, and do not see how it could be so, for me
the whole database-specific section generated by the underlying "pg_dump"
call is removed, as expected.
All is well for me, I turned the patch as ready.
While poking around the dump output, I noticed some unrelated points:
* Command "pg_dump" could tell which database is dumped in the output at
the start of the section, eg:
--
-- PostgreSQL database "foo" dump
--
Or "pg_dumpall" could issue a comment line in the output telling which
database is being considered.
* The database dumps should have an introductory comment, like there is
one for roles, eg:
--
-- Databases
--
* On extensions, the dump creates both the extension and the extension
comment. However, ISTM that the extension comment is already created by
the extension, so this is redundant:
--
-- Name: pg_dirtyread; Type: EXTENSION; Schema: -; Owner:
--
CREATE EXTENSION IF NOT EXISTS pg_dirtyread WITH SCHEMA public;
--
-- Name: EXTENSION pg_dirtyread; Type: COMMENT; Schema: -; Owner:
--
COMMENT ON EXTENSION pg_dirtyread IS 'Read dead but unvacuumed rows from table';
Maybe it should notice that the comment belongs to the extension and need
not be updated?
--
Fabien.
On Tue, Dec 25, 2018 at 09:36:05AM +0100, Fabien COELHO wrote:
I do not see Michaël's issue, and do not see how it could be so, for me the
whole database-specific section generated by the underlying "pg_dump" call
is removed, as expected.All is well for me, I turned the patch as ready.
Sorry for the noise. I have been double-checking what I said
previously and I am in the wrong.
--
-- PostgreSQL database "foo" dump
--Or "pg_dumpall" could issue a comment line in the output telling which
database is being considered.
Mentioning which database dump has been completed in the end comment
could be additionally nice.
--
Michael
Bonjour Michaï¿œl,
I do not see Michaï¿œl's issue [...]
Sorry for the noise.
No big deal!
-- PostgreSQL database "foo" dump
Or "pg_dumpall" could issue a comment line in the output telling which
database is being considered.Mentioning which database dump has been completed in the end comment
could be additionally nice.
Indeed, it should be consistent.
Joyeuses fï¿œtes !
--
Fabien.
On Tue, Dec 25, 2018 at 10:32:34AM +0100, Fabien COELHO wrote:
Joyeuses fêtes !
Merci. You too Happy New Year and Merry christmas. (Sentence valid
for all folks reading this email, as well as folks not reading it).
--
Michael
On 12/25/18 3:36 AM, Fabien COELHO wrote:
Hello Andrew,
Rebased and updated patch attached.
Here is a review of v5, sorry for the delay.
Patch applies cleanly, compiles, "make check" is ok.
I do not see Michaël's issue, and do not see how it could be so, for
me the whole database-specific section generated by the underlying
"pg_dump" call is removed, as expected.All is well for me, I turned the patch as ready.
OK, thanks.
While poking around the dump output, I noticed some unrelated points:
* Command "pg_dump" could tell which database is dumped in the output
at the start of the section, eg:--
-- PostgreSQL database "foo" dump
--Or "pg_dumpall" could issue a comment line in the output telling which
database is being considered.* The database dumps should have an introductory comment, like there
is one for roles, eg:--
-- Databases
--
I agree these are unrelated but would be nice to have. Probably having
pg_dumpall do it would be better. Do you want to do a patch for that?
* On extensions, the dump creates both the extension and the extension
comment. However, ISTM that the extension comment is already created
by the extension, so this is redundant:--
-- Name: pg_dirtyread; Type: EXTENSION; Schema: -; Owner:
--
CREATE EXTENSION IF NOT EXISTS pg_dirtyread WITH SCHEMA public;--
-- Name: EXTENSION pg_dirtyread; Type: COMMENT; Schema: -; Owner:
--
COMMENT ON EXTENSION pg_dirtyread IS 'Read dead but unvacuumed rows
from table';Maybe it should notice that the comment belongs to the extension and
need not be updated?
What if the owner had updated the comment after installing the extension?
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Hello,
While poking around the dump output, I noticed some unrelated points:
* Command "pg_dump" could tell which database is dumped in the output
Or "pg_dumpall" could issue a comment line in the output telling which
database is being considered.* The database dumps should have an introductory comment, like there
is one for rolesI agree these are unrelated but would be nice to have. Probably having
pg_dumpall do it would be better. Do you want to do a patch for that?
I do not "want" to do a patch for that:-) Anyway, a small patch is
attached which adds comments to pg_dumpall output sections.
* On extensions, the dump creates both the extension and the extension
comment. However, ISTM that the extension comment is already created
by the extension, so this is redundant:
Maybe it should notice that the comment belongs to the extension and
need not be updated?What if the owner had updated the comment after installing the extension?
Hmmm… This point could apply to anything related to an extension, which
could be altered by the owner in any way…
--
Fabien.
Attachments:
dumpall-output-more-verbose-1.patchtext/x-diff; name=dumpall-output-more-verbose-1.patchDownload
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 5176626476..021df03250 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1312,6 +1312,7 @@ dumpUserConfig(PGconn *conn, const char *username)
{
PQExpBuffer buf = createPQExpBuffer();
int count = 1;
+ bool first = true;
for (;;)
{
@@ -1333,6 +1334,14 @@ dumpUserConfig(PGconn *conn, const char *username)
if (PQntuples(res) == 1 &&
!PQgetisnull(res, 0, 0))
{
+ /* comment at section start, only if needed */
+ if (first)
+ {
+ fprintf(OPF, "--\n-- User Configurations\n--\n\n");
+ first = false;
+ }
+
+ fprintf(OPF, "--\n-- User Config \"%s\"\n--\n\n", username);
resetPQExpBuffer(buf);
makeAlterConfigCommand(conn, PQgetvalue(res, 0, 0),
"ROLE", username, NULL, NULL,
@@ -1378,6 +1387,9 @@ dumpDatabases(PGconn *conn)
"WHERE datallowconn "
"ORDER BY (datname <> 'template1'), datname");
+ if (PQntuples(res) > 0)
+ fprintf(OPF, "--\n-- Databases\n--\n\n");
+
for (i = 0; i < PQntuples(res); i++)
{
char *dbname = PQgetvalue(res, i, 0);
@@ -1391,6 +1403,8 @@ dumpDatabases(PGconn *conn)
if (verbose)
fprintf(stderr, _("%s: dumping database \"%s\"...\n"), progname, dbname);
+ fprintf(OPF, "--\n-- Database \"%s\" dump\n--\n\n", dbname);
+
/*
* We assume that "template1" and "postgres" already exist in the
* target installation. dropDBs() won't have removed them, for fear
On Thu, Dec 27, 2018 at 10:55:43PM +0100, Fabien COELHO wrote:
I do not "want" to do a patch for that:-) Anyway, a small patch is attached
which adds comments to pg_dumpall output sections.
Patch has been moved to next CF with the same status, ready for
committer. Andrew, are you planning to look at it?
--
Michael