postgres.git / summary / log / commit / refs
commit b2898baaf7e40a187de5b0134d53d944b38209cd
Author: Nathan Bossart <nathan@postgresql.org>
Date: Fri Mar 06 20:00:04 2026 +0000
pg_dumpall: Fix handling of conflicting options.
pg_dumpall is missing checks for some conflicting options,
including those passed through to pg_dump. To fix, introduce a
new function that checks whether mutually exclusive options are
set, and use that in pg_dumpall. A similar change could likely be
made for pg_dump and pg_restore, but that is left as a future
exercise.
This is arguably a bug fix, but since this might break existing
scripts, no back-patch for now.
Author: Jian He <jian.universality@gmail.com>
Co-authored-by: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Wang Peng <215722532@qq.com>
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/CACJufxFf5%3DwSv2MsuO8iZOvpLZQ1-meAMwhw7JX5gNvWo5PDug%40mail.gmail.com
src/bin/pg_dump/pg_dumpall.c | 70 +++++++++++++++--------------
src/bin/pg_dump/t/001_basic.pl | 34 +++++++++++++-
src/bin/pg_dump/t/002_pg_dump.pl | 2 -
src/bin/pg_dump/t/005_pg_dump_filterfile.pl | 4 +-
src/fe_utils/option_utils.c | 35 +++++++++++++++
src/include/fe_utils/option_utils.h | 6 +++
6 files changed, 111 insertions(+), 40 deletions(-)
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 56e25f9b156..4ded9020952 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -34,6 +34,7 @@
#include "common/string.h"
#include "connectdb.h"
#include "dumputils.h"
+#include "fe_utils/option_utils.h"
#include "fe_utils/string_utils.h"
#include "filter.h"
#include "getopt_long.h"
@@ -219,6 +220,7 @@ main(int argc, char *argv[])
bool data_only = false;
bool globals_only = false;
bool roles_only = false;
+ bool schema_only = false;
bool tablespaces_only = false;
PGconn *conn;
int encoding;
@@ -321,6 +323,7 @@ main(int argc, char *argv[])
break;
case 's':
+ schema_only = true;
appendPQExpBufferStr(pgdumpopts, " -s");
break;
@@ -418,45 +421,44 @@ main(int argc, char *argv[])
exit_nicely(1);
}
- if (database_exclude_patterns.head != NULL &&
- (globals_only || roles_only || tablespaces_only))
- {
- pg_log_error("option %s cannot be used together with %s, %s, or %s",
- "--exclude-database",
- "-g/--globals-only", "-r/--roles-only", "-t/--tablespaces-only");
- pg_log_error_hint("Try \"%s --help\" for more information.", progname);
- exit_nicely(1);
- }
-
- /* Make sure the user hasn't specified a mix of globals-only options */
- if (globals_only && roles_only)
- {
- pg_log_error("options %s and %s cannot be used together",
- "-g/--globals-only", "-r/--roles-only");
- pg_log_error_hint("Try \"%s --help\" for more information.", progname);
- exit_nicely(1);
- }
-
- if (globals_only && tablespaces_only)
- {
- pg_log_error("options %s and %s cannot be used together",
- "-g/--globals-only", "-t/--tablespaces-only");
- pg_log_error_hint("Try \"%s --help\" for more information.", progname);
- exit_nicely(1);
- }
+ /* --exclude_database is incompatible with global *-only options */
+ check_mut_excl_opts(database_exclude_patterns.head, "--exclude-database",
+ globals_only, "-g/--globals-only",
+ roles_only, "-r/--roles-only",
+ tablespaces_only, "-t/--tablespaces-only");
+
+ /* *-only options are incompatible with each other */
+ check_mut_excl_opts(data_only, "-a/--data-only",
+ globals_only, "-g/--globals-only",
+ roles_only, "-r/--roles-only",
+ schema_only, "-s/--schema-only",
+ statistics_only, "--statistics-only",
+ tablespaces_only, "-t/--tablespaces-only");
+
+ /* --no-* and *-only for same thing are incompatible */
+ check_mut_excl_opts(data_only, "-a/--data-only",
+ no_data, "--no-data");
+ check_mut_excl_opts(schema_only, "-s/--schema-only",
+ no_schema, "--no-schema");
+ check_mut_excl_opts(statistics_only, "--statistics-only",
+ no_statistics, "--no-statistics");
+
+ /* --statistics and --no-statistics are incompatible */
+ check_mut_excl_opts(with_statistics, "--statistics",
+ no_statistics, "--no-statistics");
+
+ /* --statistics is incompatible with *-only (except --statistics-only) */
+ check_mut_excl_opts(with_statistics, "--statistics",
+ data_only, "-a/--data-only",
+ globals_only, "-g/--globals-only",
+ roles_only, "-r/--roles-only",
+ schema_only, "-s/--schema-only",
+ tablespaces_only, "-t/--tablespaces-only");
if (if_exists && !output_clean)
pg_fatal("option %s requires option %s",
"--if-exists", "-c/--clean");
- if (roles_only && tablespaces_only)
- {
- pg_log_error("options %s and %s cannot be used together",
- "-r/--roles-only", "-t/--tablespaces-only");
- pg_log_error_hint("Try \"%s --help\" for more information.", progname);
- exit_nicely(1);
- }
-
/* Get format for dump. */
archDumpFormat = parseDumpFormat(format_name);
diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl
index 930ef4915c5..67131a674f4 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -240,8 +240,38 @@ command_fails_like(
# 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: error: 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'
+ qr/\Qpg_dumpall: error: options --exclude-database and -g\/--globals-only cannot be used together\E/,
+ 'pg_dumpall: options --exclude-database and -g/--globals-only cannot be used together'
+);
+
+command_fails_like(
+ [ 'pg_dumpall', '-a', '--no-data' ],
+ qr/\Qpg_dumpall: error: options -a\/--data-only and --no-data cannot be used together\E/,
+ 'pg_dumpall: options -a\/--data-only and --no-data cannot be used together'
+);
+
+command_fails_like(
+ [ 'pg_dumpall', '-s', '--no-schema' ],
+ qr/\Qpg_dumpall: error: options -s\/--schema-only and --no-schema cannot be used together\E/,
+ 'pg_dumpall: options -s\/--schema-only and --no-schema cannot be used together'
+);
+
+command_fails_like(
+ [ 'pg_dumpall', '--statistics-only', '--no-statistics' ],
+ qr/\Qpg_dumpall: error: options --statistics-only and --no-statistics cannot be used together\E/,
+ 'pg_dumpall: options --statistics-only and --no-statistics cannot be used together'
+);
+
+command_fails_like(
+ [ 'pg_dumpall', '--statistics', '--no-statistics' ],
+ qr/\Qpg_dumpall: error: options --statistics and --no-statistics cannot be used together\E/,
+ 'pg_dumpall: options --statistics-only and --no-statistics cannot be used together'
+);
+
+command_fails_like(
+ [ 'pg_dumpall', '--statistics', '--tablespaces-only' ],
+ qr/\Qpg_dumpall: error: options --statistics and -t\/--tablespaces-only cannot be used together\E/,
+ 'pg_dumpall: options --statistics and -t\/--tablespaces-only cannot be used together'
);
command_fails_like(
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 588891a62bd..e7cc998cfba 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -322,7 +322,6 @@ my %pgdump_runs = (
'--file' => "$tempdir/pg_dumpall_globals.sql",
'--globals-only',
'--no-sync',
- '--statistics',
],
},
pg_dumpall_globals_clean => {
@@ -332,7 +331,6 @@ my %pgdump_runs = (
'--globals-only',
'--clean',
'--no-sync',
- '--statistics',
],
},
pg_dumpall_dbprivs => {
diff --git a/src/bin/pg_dump/t/005_pg_dump_filterfile.pl b/src/bin/pg_dump/t/005_pg_dump_filterfile.pl
index 9c9f2baa733..b2630ef2897 100644
--- a/src/bin/pg_dump/t/005_pg_dump_filterfile.pl
+++ b/src/bin/pg_dump/t/005_pg_dump_filterfile.pl
@@ -571,8 +571,8 @@ command_fails_like(
'--filter' => "$tempdir/inputfile.txt",
'--globals-only'
],
- qr/\Qpg_dumpall: error: 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'
+ qr/\Qpg_dumpall: error: options --exclude-database and -g\/--globals-only cannot be used together\E/,
+ 'pg_dumpall: options --exclude-database and -g/--globals-only cannot be used together'
);
# Test invalid filter command
diff --git a/src/fe_utils/option_utils.c b/src/fe_utils/option_utils.c
index cc483ae176c..8d0659c1164 100644
--- a/src/fe_utils/option_utils.c
+++ b/src/fe_utils/option_utils.c
@@ -109,3 +109,38 @@ parse_sync_method(const char *optarg, DataDirSyncMethod *sync_method)
return true;
}
+
+/*
+ * Fail with appropriate error if 2 or more of the specified options are set.
+ * The first parameter is the number of arguments. Following that is an
+ * arbitrary number of bool/string pairs. The bool indicates whether the
+ * option is set, and the string is used for the error message. Note that only
+ * the first pair of enabled options we find are reported.
+ *
+ * Don't call this directly. Use the check_mut_excl_opts() macro in
+ * option_utils.h, which is the exact same except it doesn't take the first
+ * parameter (it discovers the number of arguments automagically).
+ */
+void
+check_mut_excl_opts_internal(int n,...)
+{
+ char *first = NULL;
+ va_list args;
+
+ Assert(n % 2 == 0);
+
+ va_start(args, n);
+ for (int i = 0; i < n; i += 2)
+ {
+ bool set = va_arg(args, int);
+ char *opt = va_arg(args, char *);
+
+ if (set && first)
+ pg_fatal("options %s and %s cannot be used together",
+ first, opt);
+
+ if (set)
+ first = opt;
+ }
+ va_end(args);
+}
diff --git a/src/include/fe_utils/option_utils.h b/src/include/fe_utils/option_utils.h
index 0db6e3b6e91..d975db77af2 100644
--- a/src/include/fe_utils/option_utils.h
+++ b/src/include/fe_utils/option_utils.h
@@ -24,5 +24,11 @@ extern bool option_parse_int(const char *optarg, const char *optname,
int *result);
extern bool parse_sync_method(const char *optarg,
DataDirSyncMethod *sync_method);
+extern void check_mut_excl_opts_internal(int n,...);
+
+/* see comment for check_mut_excl_opts_internal() in option_utils.c for info */
+#define check_mut_excl_opts(set, opt, ...) \
+ check_mut_excl_opts_internal(VA_ARGS_NARGS(__VA_ARGS__) + 2, \
+ set, opt, __VA_ARGS__)
#endif /* OPTION_UTILS_H */
[parent: 50ea4e09b6c2]