Here's what I have staged for commit. A few notes: * I moved the new function to option_utils.c and added a macro that takes care of figuring out the number of arguments.
* I removed the derivative flags. I know I lobbied for those, but they seemed out of place for this patch. Maybe we can add them separately. * I limited the number of new tests to one per call to check_mut_excl_opts(). I didn't see much benefit from testing everything exhaustively. -- nathan
>From ab90c166bec09af5e54f0d9308727de0f1faff48 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> Date: Tue, 3 Mar 2026 12:23:06 -0600 Subject: [PATCH v9 1/1] 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. Author: Jian He <[email protected]> Co-authored-by: Nathan Bossart <[email protected]> Reviewed-by: Wang Peng <[email protected]> Reviewed-by: Zsolt Parragi <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/CACJufxFf5%3DwSv2MsuO8iZOvpLZQ1-meAMwhw7JX5gNvWo5PDug%40mail.gmail.com Backpatch-through: 14 --- src/bin/pg_dump/pg_dumpall.c | 66 +++++++++++---------- 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, 109 insertions(+), 38 deletions(-) diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c index 1165a0f4afe..178e3f3df40 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); - } + /* --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"); - /* 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); - } + /* *-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"); - 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); - } + /* --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 a895bc314b0..78312f3ec30 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 f15bd06adcc..b06941f7088 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 */ -- 2.50.1 (Apple Git-155)
