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