On Mon, Mar 02, 2026 at 10:23:17PM +0000, Zsolt Parragi wrote: > New version looks good!
I'm not thrilled about the long list of checks. What if we added a function that could check an arbitrary number of mutually exclusive options, a bit like the attached? -- nathan
>From 4c110bd7a28d9e231fa8ccea715bbf11f9c7b384 Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Mon, 2 Mar 2026 15:17:52 +0800 Subject: [PATCH v8 1/1] pg_dumpall error out conflict options --roles-only --tablespaces-only --statistics-only --schema-only --globals-only --data-only --statistics The only permitted combination is `--statistics --statistics-only`, since pg_dump supports it as well and the semantics(meaning) of "only" are preserved. These 4 combinations should fail immediately: --schema-only --no-schema --data-only --no-data --statistics-only --no-statistics --statistics --no-statistics discussion: https://postgr.es/m/CACJufxFf5=wsv2msuo8izovplzq1-meamwhw7jx5gnvwo5p...@mail.gmail.com commitfest entry: https://commitfest.postgresql.org/patch/6459 --- src/bin/pg_dump/dumputils.c | 21 ++++ src/bin/pg_dump/dumputils.h | 1 + src/bin/pg_dump/pg_dumpall.c | 80 ++++++------ src/bin/pg_dump/t/001_basic.pl | 128 +++++++++++++++++++- src/bin/pg_dump/t/002_pg_dump.pl | 2 - src/bin/pg_dump/t/005_pg_dump_filterfile.pl | 2 +- 6 files changed, 190 insertions(+), 44 deletions(-) diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c index 5bc77fed974..21c7a20b310 100644 --- a/src/bin/pg_dump/dumputils.c +++ b/src/bin/pg_dump/dumputils.c @@ -1000,3 +1000,24 @@ valid_restrict_key(const char *restrict_key) restrict_key[0] != '\0' && strspn(restrict_key, restrict_chars) == strlen(restrict_key); } + +void +CheckMutuallyExclusiveOpts(int n,...) +{ + char *first = NULL; + va_list args; + + 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) + first = opt; + else if (set) + pg_fatal("options %s and %s cannot be used together", + first, opt); + } + va_end(args); +} diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h index d231ce1d654..c386558049c 100644 --- a/src/bin/pg_dump/dumputils.h +++ b/src/bin/pg_dump/dumputils.h @@ -68,5 +68,6 @@ extern void create_or_open_dir(const char *dirname); extern char *generate_restrict_key(void); extern bool valid_restrict_key(const char *restrict_key); +extern void CheckMutuallyExclusiveOpts(int n,...); #endif /* DUMPUTILS_H */ diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c index 1165a0f4afe..f30e7cecfaa 100644 --- a/src/bin/pg_dump/pg_dumpall.c +++ b/src/bin/pg_dump/pg_dumpall.c @@ -217,6 +217,7 @@ main(int argc, char *argv[]) const char *format_name = "p"; trivalue prompt_password = TRI_DEFAULT; bool data_only = false; + bool schema_only = false; bool globals_only = false; bool roles_only = false; bool tablespaces_only = false; @@ -226,6 +227,9 @@ main(int argc, char *argv[]) ret; int optindex; DumpOptions dopt; + bool dump_DBs; + bool dump_roles; + bool dump_tablespaces; pg_logging_init(argv[0]); pg_logging_set_level(PG_LOG_WARNING); @@ -321,6 +325,7 @@ main(int argc, char *argv[]) break; case 's': + schema_only = true; appendPQExpBufferStr(pgdumpopts, " -s"); break; @@ -418,45 +423,36 @@ 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); - } + CheckMutuallyExclusiveOpts(4, data_only, "-a/--data-only", + no_data, "--no-data"); + CheckMutuallyExclusiveOpts(4, schema_only, "-s/--schema-only", + no_schema, "--no-schema"); + CheckMutuallyExclusiveOpts(4, statistics_only, "--statistics-only", + no_statistics, "--no-statistics"); + CheckMutuallyExclusiveOpts(4, with_statistics, "--statistics", + no_statistics, "--no-statistics"); + CheckMutuallyExclusiveOpts(12, globals_only, "-g/--globals-only", + roles_only, "-r/--roles-only", + tablespaces_only, "-t/--tablespaces-only", + data_only, "-a/--data-only", + schema_only, "-s/--schema-only", + statistics_only, "--statistics-only"); + CheckMutuallyExclusiveOpts(12, + globals_only, "-g/--globals-only", + roles_only, "-r/--roles-only", + tablespaces_only, "-t/--tablespaces-only", + schema_only, "-s/--schema-only", + data_only, "-a/--data-only", + with_statistics, "--statistics"); + CheckMutuallyExclusiveOpts(8, database_exclude_patterns.head != NULL, "--exclude-database", + globals_only, "-g/--globals-only", + roles_only, "-r/--roles-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); @@ -766,6 +762,10 @@ main(int argc, char *argv[]) fprintf(OPF, "\n"); } + dump_DBs = !globals_only && !roles_only && !tablespaces_only; + dump_tablespaces = !roles_only && !no_tablespaces; + dump_roles = !tablespaces_only; + if (!data_only && !statistics_only && !no_schema) { /* @@ -782,13 +782,13 @@ main(int argc, char *argv[]) */ if (output_clean || archDumpFormat != archNull) { - if (!globals_only && !roles_only && !tablespaces_only) + if (dump_DBs) dropDBs(conn); - if (!roles_only && !no_tablespaces) + if (dump_tablespaces) dropTablespaces(conn); - if (!tablespaces_only) + if (dump_roles) dropRoles(conn); } @@ -796,7 +796,7 @@ main(int argc, char *argv[]) * Now create objects as requested. Be careful that option logic here * is the same as for drops above. */ - if (!tablespaces_only) + if (dump_roles) { /* Dump roles (users) */ dumpRoles(conn); @@ -810,7 +810,7 @@ main(int argc, char *argv[]) } /* Dump tablespaces */ - if (!roles_only && !no_tablespaces) + if (dump_tablespaces) dumpTablespaces(conn); } @@ -823,7 +823,7 @@ main(int argc, char *argv[]) fprintf(OPF, "\\unrestrict %s\n\n", restrict_key); } - if (!globals_only && !roles_only && !tablespaces_only) + if (dump_DBs) dumpDatabases(conn); if (archDumpFormat == archNull) diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl index a895bc314b0..abb75ba5746 100644 --- a/src/bin/pg_dump/t/001_basic.pl +++ b/src/bin/pg_dump/t/001_basic.pl @@ -220,12 +220,138 @@ command_fails_like( 'pg_dumpall: options -g/--globals-only and -t/--tablespaces-only cannot be used together' ); +command_fails_like( + [ 'pg_dumpall', '--data-only', '--no-data'], + qr/\Qpg_dumpall: error: options -a\/--data-only and --no-data cannot be used together\E/, + 'pg_dumpall: error: options --no-data and -a/--data-only cannot be used together' +); + +command_fails_like( + [ 'pg_dumpall', '--schema-only', '--no-schema'], + qr/\Qpg_dumpall: error: options -s\/--schema-only and --no-schema cannot be used together\E/, + 'pg_dumpall: error: 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: error: 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: error: options --statistics and --no-statistics cannot be used together' +); + +command_fails_like( + [ 'pg_dumpall', '-g', '--statistics' ], + qr/\Qpg_dumpall: error: options -g\/--globals-only and --statistics cannot be used together\E/, + 'pg_dumpall: error: options -g/--globals-only and --statistics cannot be used together' +); + +command_fails_like( + [ 'pg_dumpall', '-g', '--statistics-only' ], + qr/\Qpg_dumpall: error: options -g\/--globals-only and --statistics-only cannot be used together\E/, + 'pg_dumpall: error: options -g/--globals-only and --statistics-only cannot be used together' +); + +command_fails_like( + [ 'pg_dumpall', '-g', '-s' ], + qr/\Qpg_dumpall: error: options -g\/--globals-only and -s\/--schema-only cannot be used together\E/, + 'pg_dumpall: error: options -g/--globals-only and -s/--schema-only cannot be used together' +); + +command_fails_like( + [ 'pg_dumpall', '-g', '-a' ], + qr/\Qpg_dumpall: error: options -g\/--globals-only and -a\/--data-only cannot be used together\E/, + 'pg_dumpall: error: options -g/--globals-only and -a/--data-only cannot be used together' +); + command_fails_like( [ 'pg_dumpall', '-r', '-t' ], qr/\Qpg_dumpall: error: options -r\/--roles-only and -t\/--tablespaces-only cannot be used together\E/, 'pg_dumpall: options -r/--roles-only and -t/--tablespaces-only cannot be used together' ); +command_fails_like( + [ 'pg_dumpall', '-r', '-s' ], + qr/\Qpg_dumpall: error: options -r\/--roles-only and -s\/--schema-only cannot be used together\E/, + 'pg_dumpall: error: options -r/--roles-only and -s/--schema-only cannot be used together' +); + +command_fails_like( + [ 'pg_dumpall', '-r', '--statistics' ], + qr/\Qpg_dumpall: error: options -r\/--roles-only and --statistics cannot be used together\E/, + 'pg_dumpall: error: options -r/--roles-only and --statistics cannot be used together' +); + +command_fails_like( + [ 'pg_dumpall', '-r', '--statistics-only' ], + qr/\Qpg_dumpall: error: options -r\/--roles-only and --statistics-only cannot be used together\E/, + 'pg_dumpall: error: options -r/--roles-only and --statistics-only cannot be used together' +); + +command_fails_like( + [ 'pg_dumpall', '-r', '-a' ], + qr/\Qpg_dumpall: error: options -r\/--roles-only and -a\/--data-only cannot be used together\E/, + 'pg_dumpall: error: options -r/--roles-only and -a/--data-only cannot be used together' +); + +command_fails_like( + [ 'pg_dumpall', '-t', '-s' ], + qr/\Qpg_dumpall: error: options -t\/--tablespaces-only and -s\/--schema-only cannot be used together\E/, + 'pg_dumpall: error: options -t/--tablespaces-only and -s/--schema-only cannot be used together' +); + +command_fails_like( + [ 'pg_dumpall', '-t', '--statistics' ], + qr/\Qpg_dumpall: error: options -t\/--tablespaces-only and --statistics cannot be used together\E/, + 'pg_dumpall: error: options -t/--tablespaces-only and --statistics cannot be used together' +); + +command_fails_like( + [ 'pg_dumpall', '-t', '--statistics-only'], + qr/\Qpg_dumpall: error: options -t\/--tablespaces-only and --statistics-only cannot be used together\E/, + 'pg_dumpall: error: options -t/--tablespaces-only and --statistics-only cannot be used together' +); + +command_fails_like( + [ 'pg_dumpall', '-t', '-a'], + qr/\Qpg_dumpall: error: options -t\/--tablespaces-only and -a\/--data-only cannot be used together\E/, + 'pg_dumpall: error: options -t/--tablespaces-only and -a/--data-only cannot be used together' +); + +command_fails_like( + [ 'pg_dumpall', '-a', '-s' ], + qr/\Qpg_dumpall: error: options -a\/--data-only and -s\/--schema-only cannot be used together\E/, + 'pg_dumpall: error: options -a/--data-only and -s/--schema-only cannot be used together' +); + +command_fails_like( + [ 'pg_dumpall', '-a', '--statistics' ], + qr/\Qpg_dumpall: error: options -a\/--data-only and --statistics cannot be used together\E/, + 'pg_dumpall: error: options -a/--data-only and --statistics cannot be used together' +); + +command_fails_like( + [ 'pg_dumpall', '-a', '--statistics-only' ], + qr/\Qpg_dumpall: error: options -a\/--data-only and --statistics-only cannot be used together\E/, + 'pg_dumpall: error: options -a/--data-only and --statistics-only cannot be used together' +); + +command_fails_like( + [ 'pg_dumpall', '-s', '--statistics' ], + qr/\Qpg_dumpall: error: options -s\/--schema-only and --statistics cannot be used together\E/, + 'pg_dumpall: error: options -s/--schema-only and --statistics cannot be used together' +); + +command_fails_like( + [ 'pg_dumpall', '-s', '--statistics-only' ], + qr/\Qpg_dumpall: error: options -s\/--schema-only and --statistics-only cannot be used together\E/, + 'pg_dumpall: error: options /-s\/--schema-only and --statistics-only cannot be used together' +); + command_fails_like( [ 'pg_dumpall', '--if-exists' ], qr/\Qpg_dumpall: error: option --if-exists requires option -c\/--clean\E/, @@ -240,7 +366,7 @@ 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/, + qr/\Qpg_dumpall: error: options --exclude-database and -g\/--globals-only cannot be used together\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 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..9cc88247037 100644 --- a/src/bin/pg_dump/t/005_pg_dump_filterfile.pl +++ b/src/bin/pg_dump/t/005_pg_dump_filterfile.pl @@ -571,7 +571,7 @@ 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/, + qr/\Qpg_dumpall: error: options --exclude-database and -g\/--globals-only cannot be used together\E/, 'pg_dumpall: option --exclude-database cannot be used together with -g/--globals-only' ); -- 2.50.1 (Apple Git-155)
