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
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,
