On Thu, Feb 5, 2026 at 2:33 AM Nathan Bossart <[email protected]> wrote:
> I'm not following your objection here. If anything, such a change would
> reduce complexity. For example, we currently use the following check in
> multiple places to decide whether to drop/drump databases:
>
> if (!globals_only && !roles_only && !tablespaces_only)
>
> If we created a derivative flag like this:
>
> shouldDumpDBs = !globals_only && !roles_only && !tablespaces_only;
>
> We could then decide whether to do database things like this:
>
> if (shouldDumpDBs)
> dumpDatabases(conn);
>
> This has the added benefit of simplifying future patches that add new -only
> options. If/when that happens, we'd just add it to the line that sets
> shouldDumpDBs, whereas today we'd need to go through the rest of the code
> and update multiple conditions. Not to mention the readability
> improvements...
I thought you meant to add a new field to DumpOptions.
I've added 3 bool variables: shouldDumpDBs, shouldDumpTablespaces,
shouldDumpRoles.
shouldDumpDBs = !globals_only && !roles_only && !tablespaces_only;
shouldDumpTablespaces = !roles_only && !no_tablespaces &&
!data_only && !schema_only && !statistics_only;
shouldDumpRoles = !tablespaces_only && !data_only && !schema_only
&& !statistics_only;
pg_dumpall --statistics
will dump global objects, data, schema, and statistics.
Which is correct, I think.
>
> + /* reject conflicting "-only" options */
> + if (globals_only && with_statistics)
> + pg_fatal("options %s and %s cannot be used together",
> + "-g/--globals-only", "--statistics");
> + if (globals_only && statistics_only)
> + pg_fatal("options %s and %s cannot be used together",
> + "-g/--globals-only", "--statistics-only");
>
> As before, I think we should integrate the new conflicting option handling
> into the existing section that does this sort of thing. We should also
> make sure the handling is the same. The existing code uses pg_log_error(),
> pg_log_error_hint(), and exit_nicely(), while the patch uses pg_fatal().
>
Adding a pg_log_error_hint would likely be helpful, since the reason
for the failure is not very intuitive,
The attached patch also addresses the points mentioned by Zsolt Parragi.
I just found out
pg_dumpall --no-data --data-only
will not immediately fail, it will fail during pg_dumpall call pg_dump.
not sure if this is ok or not.
--
jian
https://www.enterprisedb.com/
From 332c8353977439e0ec12fb753bfb5c3cc6298b98 Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Thu, 5 Feb 2026 11:25:00 +0800
Subject: [PATCH v3 1/1] pg_dumpall error out conflict options
make the below command error out:
pg_dumpall --globals-only --statistics-only
pg_dumpall --globals-only --statistics
pg_dumpall --roles-only --statistics-only
pg_dumpall --roles-only --data-only
pg_dumpall --roles-only --schema-only
pg_dumpall --roles-only --statistics
pg_dumpall --tablespaces-only --statistics-only
pg_dumpall --tablespaces-only --data-only
pg_dumpall --tablespaces-only --schema-only
pg_dumpall --tablespaces-only --statistics
pg_dumpall: --statistics-only, --data-only, --schema-only
implies don't dump globals objects(roles, tablespaces).
pg_dumpall: --globals-only, --roles-only, --tablespaces-only implies don't dump databases.
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 | 157 ++++++++++++++++++++++++-------
src/bin/pg_dump/t/001_basic.pl | 60 ++++++++++++
src/bin/pg_dump/t/002_pg_dump.pl | 2 -
3 files changed, 182 insertions(+), 37 deletions(-)
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 30fecd0c252..f6b22031151 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 shouldDumpDBs;
+ bool shouldDumpRoles;
+ bool shouldDumpTablespaces;
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;
@@ -422,6 +427,22 @@ main(int argc, char *argv[])
exit_nicely(1);
}
+ if (globals_only && with_statistics)
+ {
+ pg_log_error("options %s and %s cannot be used together",
+ "-g/--globals-only", "--statistics");
+ pg_log_error_hint("Try \"%s --help\" for more information.", progname);
+ exit_nicely(1);
+ }
+
+ if (globals_only && statistics_only)
+ {
+ pg_log_error("options %s and %s cannot be used together",
+ "-g/--globals-only", "--statistics-only");
+ pg_log_error_hint("Try \"%s --help\" for more information.", progname);
+ exit_nicely(1);
+ }
+
if (if_exists && !output_clean)
pg_fatal("option %s requires option %s",
"--if-exists", "-c/--clean");
@@ -434,6 +455,71 @@ main(int argc, char *argv[])
exit_nicely(1);
}
+ /* reject conflicting "-only" options */
+ if (data_only && roles_only)
+ {
+ pg_log_error("options %s and %s cannot be used together",
+ "-a/--data-only", "-r/--roles-only");
+ pg_log_error_hint("Try \"%s --help\" for more information.", progname);
+ exit_nicely(1);
+ }
+
+ if (schema_only && roles_only)
+ {
+ pg_log_error("options %s and %s cannot be used together",
+ "-s/--schema-only", "-r/--roles-only");
+ pg_log_error_hint("Try \"%s --help\" for more information.", progname);
+ exit_nicely(1);
+ }
+
+ if (with_statistics && roles_only)
+ {
+ pg_log_error("options %s and %s cannot be used together",
+ "--statistics", "-r/--roles-only");
+ pg_log_error_hint("Try \"%s --help\" for more information.", progname);
+ exit_nicely(1);
+ }
+
+ if (statistics_only && roles_only)
+ {
+ pg_log_error("options %s and %s cannot be used together",
+ "--statistics-only", "-r/--roles-only");
+ pg_log_error_hint("Try \"%s --help\" for more information.", progname);
+ exit_nicely(1);
+ }
+
+ if (data_only && tablespaces_only)
+ {
+ pg_log_error("options %s and %s cannot be used together",
+ "-a/--data-only", "-t/--tablespaces-only");
+ pg_log_error_hint("Try \"%s --help\" for more information.", progname);
+ exit_nicely(1);
+ }
+
+ if (schema_only && tablespaces_only)
+ {
+ pg_log_error("options %s and %s cannot be used together",
+ "-s/--schema-only", "-t/--tablespaces-only");
+ pg_log_error_hint("Try \"%s --help\" for more information.", progname);
+ exit_nicely(1);
+ }
+
+ if (with_statistics && tablespaces_only)
+ {
+ pg_log_error("options %s and %s cannot be used together",
+ "--statistics", "-t/--tablespaces-only");
+ pg_log_error_hint("Try \"%s --help\" for more information.", progname);
+ exit_nicely(1);
+ }
+
+ if (statistics_only && tablespaces_only)
+ {
+ pg_log_error("options %s and %s cannot be used together",
+ "--statistics-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,55 +709,56 @@ main(int argc, char *argv[])
fprintf(OPF, "SET standard_conforming_strings = on;\n");
fprintf(OPF, "\n");
- if (!data_only && !statistics_only && !no_schema)
- {
- /*
- * If asked to --clean, do that first. We can avoid detailed
- * dependency analysis because databases never depend on each other,
- * and tablespaces never depend on each other. Roles could have
- * grants to each other, but DROP ROLE will clean those up silently.
- */
- if (output_clean)
- {
- if (!globals_only && !roles_only && !tablespaces_only)
- dropDBs(conn);
+ shouldDumpDBs = !globals_only && !roles_only && !tablespaces_only;
+ shouldDumpTablespaces = !roles_only && !no_tablespaces && !data_only && !schema_only && !statistics_only;
+ shouldDumpRoles = !tablespaces_only && !data_only && !schema_only && !statistics_only;
- if (!roles_only && !no_tablespaces)
- dropTablespaces(conn);
+ /*
+ * If asked to --clean, do that first. We can avoid detailed dependency
+ * analysis because databases never depend on each other, and tablespaces
+ * never depend on each other. Roles could have grants to each other, but
+ * DROP ROLE will clean those up silently.
+ */
+ if (output_clean)
+ {
+ if (shouldDumpDBs)
+ dropDBs(conn);
- if (!tablespaces_only)
- dropRoles(conn);
- }
+ if (shouldDumpTablespaces)
+ dropTablespaces(conn);
- /*
- * Now create objects as requested. Be careful that option logic here
- * is the same as for drops above.
- */
- if (!tablespaces_only)
- {
- /* Dump roles (users) */
- dumpRoles(conn);
+ if (shouldDumpRoles)
+ dropRoles(conn);
+ }
- /* Dump role memberships */
- dumpRoleMembership(conn);
+ /*
+ * Now create objects as requested. Be careful that option logic here is
+ * the same as for drops above.
+ */
+ if (shouldDumpRoles)
+ {
+ /* Dump roles (users) */
+ dumpRoles(conn);
- /* Dump role GUC privileges */
- if (server_version >= 150000 && !skip_acls)
- dumpRoleGUCPrivs(conn);
- }
+ /* Dump role memberships */
+ dumpRoleMembership(conn);
- /* Dump tablespaces */
- if (!roles_only && !no_tablespaces)
- dumpTablespaces(conn);
+ /* Dump role GUC privileges */
+ if (server_version >= 150000 && !skip_acls)
+ dumpRoleGUCPrivs(conn);
}
+ /* Dump tablespaces */
+ if (shouldDumpTablespaces)
+ dumpTablespaces(conn);
+
/*
* Exit restricted mode just before dumping the databases. pg_dump will
* handle entering restricted mode again as appropriate.
*/
fprintf(OPF, "\\unrestrict %s\n\n", restrict_key);
- if (!globals_only && !roles_only && !tablespaces_only)
+ if (shouldDumpDBs)
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..99db465c9ed 100644
--- a/src/bin/pg_dump/t/001_basic.pl
+++ b/src/bin/pg_dump/t/001_basic.pl
@@ -226,6 +226,66 @@ command_fails_like(
'pg_dumpall: options -r/--roles-only and -t/--tablespaces-only 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', '--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', '-a', '-r' ],
+ qr/\Qpg_dumpall: error: options -a\/--data-only and -r\/--roles-only cannot be used together\E/,
+ 'pg_dumpall: error: options -a/--data-only and -r/--roles-only cannot be used together'
+);
+
+command_fails_like(
+ [ 'pg_dumpall', '-s', '-r' ],
+ qr/\Qpg_dumpall: error: options -s\/--schema-only and -r\/--roles-only cannot be used together\E/,
+ 'pg_dumpall: error: options -s/--schema-only and -r/--roles-only cannot be used together'
+);
+
+command_fails_like(
+ [ 'pg_dumpall', '--statistics', '-r' ],
+ qr/\Qpg_dumpall: error: options --statistics and -r\/--roles-only cannot be used together\E/,
+ 'pg_dumpall: error: options --statistics and -r/--roles-only cannot be used together'
+);
+
+command_fails_like(
+ [ 'pg_dumpall', '--statistics-only', '-r' ],
+ qr/\Qpg_dumpall: error: options --statistics-only and -r\/--roles-only cannot be used together\E/,
+ 'pg_dumpall: error: options --statistics-only and -r/--roles-only cannot be used together'
+);
+
+command_fails_like(
+ [ 'pg_dumpall', '-a', '-t' ],
+ qr/\Qpg_dumpall: error: options -a\/--data-only and -t\/--tablespaces-only cannot be used together\E/,
+ 'pg_dumpall: error: options -a/--data-only and -t/--tablespaces-only cannot be used together'
+);
+
+command_fails_like(
+ [ 'pg_dumpall', '-s', '-t' ],
+ qr/\Qpg_dumpall: error: options -s\/--schema-only and -t\/--tablespaces-only cannot be used together\E/,
+ 'pg_dumpall: error: options -s/--schema-only and -t/--tablespaces-only cannot be used together'
+);
+
+command_fails_like(
+ [ 'pg_dumpall', '--statistics', '-t' ],
+ qr/\Qpg_dumpall: error: options --statistics and -t\/--tablespaces-only cannot be used together\E/,
+ 'pg_dumpall: error: options --statistics and -t/--tablespaces-only cannot be used together'
+);
+
+command_fails_like(
+ [ 'pg_dumpall', '--statistics-only', '-t'],
+ qr/\Qpg_dumpall: error: options --statistics-only and -t\/--tablespaces-only cannot be used together\E/,
+ 'pg_dumpall: error: options --statistics-only and -t/--tablespaces-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 a8dcc2b5c75..340cf953a60 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
