On Tue, Feb 10, 2026 at 3:47 AM Zsolt Parragi <[email protected]> wrote: > > > Is there a reason we need to duplicate these checks in pg_dumpall when they > > are already handled by pg_dump? > > Mainly I think it would be a nicer user experience to fail early > without generating additional output other than the error message > (currently it writes out 26 lines before the error), but there are > also two specific reasons why it would be an improvement: > > * "--schema-only --no-schema" is already a contradiction before > pg_dumpall calls pg_dump: should it print out roles/tablespaces or > not? (it doesn't) > * if you specify "pg_dumpall --data-only -no-data -f dump.sql", or > redirect stdout to a file, it writes out a partial dump before > failing, and leaves it there. Users should check error messages and > exit codes, but the file is still there and could cause accidents. 3 > simple checks could prevent this.
OK. The attached v6 added these 3 "--only" and "--no" checks, along with related tests. -- jian https://www.enterprisedb.com/
From a070fff115d1f4f1d1101062e429054a71321c68 Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Tue, 24 Feb 2026 09:08:55 +0800 Subject: [PATCH v6 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. there 3 combination should fail immediately: --schema-only --no-schema --data-only --no-data --statistics-only --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/pg_dumpall.c | 105 ++++++++++++++++++++++----- src/bin/pg_dump/t/001_basic.pl | 120 +++++++++++++++++++++++++++++++ src/bin/pg_dump/t/002_pg_dump.pl | 2 - 3 files changed, 206 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c index 98389d2034c..02190910d78 100644 --- a/src/bin/pg_dump/pg_dumpall.c +++ b/src/bin/pg_dump/pg_dumpall.c @@ -198,6 +198,7 @@ main(int argc, char *argv[]) char *use_role = NULL; const char *dumpencoding = NULL; trivalue prompt_password = TRI_DEFAULT; + bool schema_only = false; bool data_only = false; bool globals_only = false; bool roles_only = false; @@ -207,6 +208,9 @@ main(int argc, char *argv[]) int c, ret; int optindex; + bool dump_DBs; + bool dump_roles; + bool dump_tablespaces; pg_logging_init(argv[0]); pg_logging_set_level(PG_LOG_WARNING); @@ -299,6 +303,7 @@ main(int argc, char *argv[]) break; case 's': + schema_only = true; appendPQExpBufferStr(pgdumpopts, " -s"); break; @@ -405,19 +410,85 @@ main(int argc, char *argv[]) exit_nicely(1); } - /* Make sure the user hasn't specified a mix of globals-only options */ - if (globals_only && roles_only) + if (data_only && no_data) + pg_fatal("options %s and %s cannot be used together", + "-a/--data-only", "--no-data"); + + if (schema_only && no_schema) + pg_fatal("options %s and %s cannot be used together", + "-s/--schema-only", "--no-schema"); + + if (statistics_only && no_statistics) + pg_fatal("options %s and %s cannot be used together", + "--statistics-only", "--no-statistics"); + + /* + * There must be at most one "only" option. --statistics is compatible + * only with --statistics-only. + */ + if (globals_only && + (roles_only || tablespaces_only || with_statistics || statistics_only || schema_only || data_only)) { pg_log_error("options %s and %s cannot be used together", - "-g/--globals-only", "-r/--roles-only"); + "-g/--globals-only", + roles_only ? "-r/--roles-only" : + tablespaces_only ? "-t/--tablespaces-only" : + with_statistics ? "--statistics" : + statistics_only ? "--statistics-only" : + schema_only ? "-s/--schema-only" : + "-a/--data-only"); + pg_log_error_hint("Try \"%s --help\" for more information.", progname); exit_nicely(1); } - if (globals_only && tablespaces_only) + if (roles_only && + (tablespaces_only || schema_only || with_statistics || statistics_only || data_only)) { pg_log_error("options %s and %s cannot be used together", - "-g/--globals-only", "-t/--tablespaces-only"); + "-r/--roles-only", + tablespaces_only ? "-t/--tablespaces-only" : + schema_only ? "-s/--schema-only" : + with_statistics ? "--statistics" : + statistics_only ? "--statistics-only" : + "-a/--data-only"); + + pg_log_error_hint("Try \"%s --help\" for more information.", progname); + exit_nicely(1); + } + + if (tablespaces_only && + (schema_only || with_statistics || statistics_only || data_only)) + { + pg_log_error("options %s and %s cannot be used together", + "-t/--tablespaces-only", + schema_only ? "-s/--schema-only" : + with_statistics ? "--statistics" : + statistics_only ? "--statistics-only" : + "-a/--data-only"); + + pg_log_error_hint("Try \"%s --help\" for more information.", progname); + exit_nicely(1); + } + + if (data_only && (schema_only || with_statistics || statistics_only)) + { + pg_log_error("options %s and %s cannot be used together", + "-a/--data-only", + schema_only ? "-s/--schema-only" : + statistics_only ? "--statistics-only" : + "--statistics"); + pg_log_error_hint("Try \"%s --help\" for more information.", progname); + exit_nicely(1); + } + + if (schema_only && (with_statistics || statistics_only)) + { + pg_log_error("options %s and %s cannot be used together", + "-s/--schema-only", + statistics_only ? "--statistics-only" : + "--statistics"); + pg_log_error_hint("Try \"%s --help\" for more information.", progname); exit_nicely(1); } @@ -426,14 +497,6 @@ main(int argc, char *argv[]) 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); - } - /* * If password values are not required in the dump, switch to using * pg_roles which is equally useful, just more likely to have unrestricted @@ -623,6 +686,10 @@ main(int argc, char *argv[]) fprintf(OPF, "SET standard_conforming_strings = on;\n"); 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) { /* @@ -633,13 +700,13 @@ main(int argc, char *argv[]) */ if (output_clean) { - 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); } @@ -647,7 +714,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); @@ -661,7 +728,7 @@ main(int argc, char *argv[]) } /* Dump tablespaces */ - if (!roles_only && !no_tablespaces) + if (dump_tablespaces) dumpTablespaces(conn); } @@ -671,7 +738,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); PQfinish(conn); diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl index ab9310eb42b..8de83e6e332 100644 --- a/src/bin/pg_dump/t/001_basic.pl +++ b/src/bin/pg_dump/t/001_basic.pl @@ -220,12 +220,132 @@ 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', '-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 /--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/, diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index 8a21a5a5369..2dfc2d2f2a1 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 => { -- 2.34.1
