pg_dumpall --roles-only interact with other options
hi.
pg_dumpall --verbose --roles-only --no-schema --file=1.sql
pg_dumpall --verbose --roles-only --no-data --file=2.sql
pg_dumpall --verbose --roles-only --no-statistics --file=3.sql
pg_dumpall --verbose --roles-only --statistics-only --file=4.sql
pg_dumpall --verbose --roles-only --data-only --file=5.sql
pg_dumpall --verbose --roles-only --schema-only --file=6.sql
What should we expect for the above commands?
the current behavior is not good, i think, some even do not dump the
roles command.
I would expect the last three commands to raise errors, while the first three
should simply ignore the options (--no-schema, --no-data, --no-statistics).
This situation also happens to another pg_duampall option: --tablespaces-only.
what do you think?
On Sat, Jan 31, 2026 at 11:50:50AM +0800, jian he wrote:
pg_dumpall --verbose --roles-only --no-schema --file=1.sql
pg_dumpall --verbose --roles-only --no-data --file=2.sql
pg_dumpall --verbose --roles-only --no-statistics --file=3.sql
These seem permissible to me. The --no-* options are redundant, but the
user intent seems clear.
pg_dumpall --verbose --roles-only --statistics-only --file=4.sql
pg_dumpall --verbose --roles-only --data-only --file=5.sql
pg_dumpall --verbose --roles-only --schema-only --file=6.sql[...]
This situation also happens to another pg_duampall option:
--tablespaces-only.
Yeah, conflicting --*-only options should probably cause errors, like we do
for pg_dump.
--
nathan
hi.
please check the attached.
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 --globals-only --statistics
the above will all error out.
``pg_dumpall --globals-only --statistics`` should error,
the HEAD behavior does not respect "--statistics", maybe we can make
it not error out, but
that would contradict the meaning of "--globals-only", i think.
pg_dumpall --roles-only --no-schema --file=1.sql
pg_dumpall --roles-only --no-data --file=2.sql
pg_dumpall --roles-only --no-statistics --file=3.sql
pg_dumpall --tablespaces-only --no-schema --file=1.sql
pg_dumpall --tablespaces-only --no-data --file=2.sql
pg_dumpall --tablespaces-only --no-statistics --file=3.sql
The items listed above respect the 'only' option but ignore the 'no' option."
Attachments:
v1-0001-pg_dumpall-only-option-fix.patchtext/x-patch; charset=US-ASCII; name=v1-0001-pg_dumpall-only-option-fix.patchDownload+77-44
On Tue, Feb 03, 2026 at 10:15:35AM +0800, jian he wrote:
please check the attached.
Thanks.
- {"statistics", no_argument, &with_statistics, 1}, - {"statistics-only", no_argument, &statistics_only, 1}, + {"statistics", no_argument, NULL, 10}, + {"statistics-only", no_argument, NULL, 11},
nitpick: I don't totally disagree with these changes, but they are
unrelated to the patch at hand, so I think we'd better leave them out.
+ /* reject conflicting "-only" options */ + if (globals_only && with_statistics) + pg_fatal("options %s and %s cannot be used together", + "-g/--globals-only", "--statistics"); + + if (data_only && roles_only) + pg_fatal("options %s and %s cannot be used together", + "-a/--data-only", "-r/--roles-only"); [...] + + if (data_only && tablespaces_only) + pg_fatal("options %s and %s cannot be used together", + "-a/--data-only", "-t/--tablespaces-only"); [...]
Could we integrate this into the existing handling for conflicting options
a few lines above this point?
- if (!data_only && !statistics_only && !no_schema)
I wonder if we ought to create "derivative flags" like we did for pg_dump
in commit 96a81c1be9. That could make some of this stuff easier to
maintain and to follow.
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 => {
Could you add some new tests for the conflicting options?
--
nathan
jian he 写于 2026/2/3 10:15:
hi.
please check the attached.
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 --globals-only --statisticsthe above will all error out.
``pg_dumpall --globals-only --statistics`` should error,
the HEAD behavior does not respect "--statistics", maybe we can make
it not error out, but
that would contradict the meaning of "--globals-only", i think.pg_dumpall --roles-only --no-schema --file=1.sql
pg_dumpall --roles-only --no-data --file=2.sql
pg_dumpall --roles-only --no-statistics --file=3.sql
pg_dumpall --tablespaces-only --no-schema --file=1.sql
pg_dumpall --tablespaces-only --no-data --file=2.sql
pg_dumpall --tablespaces-only --no-statistics --file=3.sqlThe items listed above respect the 'only' option but ignore the 'no' option."
Hi,
I reviewed and tested this patch. I noticed that:
pg_dumpall --globals-only --statistics ----> error
pg_dumpall --globals-only --statistics-only ----> pass
maybe there is inconsistent for *-only options
is that intentional?
Best regards,
--
wangpeng
On Wed, Feb 4, 2026 at 9:56 AM wangpeng <215722532@qq.com> wrote:
Hi,
I reviewed and tested this patch. I noticed that:
pg_dumpall --globals-only --statistics ----> error
pg_dumpall --globals-only --statistics-only ----> pass
maybe there is inconsistent for *-only options
is that intentional?
Thanks for pointing this out.
It should fail too. I missed this combination.
The attached v2 should be bullet-proof.
On Wed, Feb 4, 2026 at 5:25 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
I wonder if we ought to create "derivative flags" like we did for pg_dump
in commit 96a81c1be9. That could make some of this stuff easier to
maintain and to follow.
https://git.postgresql.org/cgit/postgresql.git/commit/?id=96a81c1be929d122719bd289f6e24824f37e1ff6
added new fields to RestoreOptions and DumpOptions.
These global objects dump(roles, tablespaces) are not directly related to
pg_restore for now, pg_restore does not support options like --roles-only
or --tablespaces-only. Creating "derivative flags" requires careful
consideration of their default values, which adds complexity for relatively
little benefit. Overall we don't need to implement similar logic now, i think.
commitgest entry: https://commitfest.postgresql.org/patch/6459
Attachments:
v2-0001-pg_dumpall-error-out-conflict-option.patchtext/x-patch; charset=US-ASCII; name=v2-0001-pg_dumpall-error-out-conflict-option.patchDownload+130-40
On Wed, Feb 04, 2026 at 04:14:59PM +0800, jian he wrote:
These global objects dump(roles, tablespaces) are not directly related to
pg_restore for now, pg_restore does not support options like --roles-only
or --tablespaces-only.
I'm suggesting adding derivative flags to pg_dumpall, not pg_restore.
Creating "derivative flags" requires careful
consideration of their default values, which adds complexity for relatively
little benefit. Overall we don't need to implement similar logic now, i think.
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...
+ /* 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().
--
nathan
Hello!
Should these work? (currently these don't result in errors, but
doesn't seem to be useful in practice)
pg_dumpall --globals-only --no-schema
pg_dumpall --globals-only --data-only
Also previously the code had a check that certain flags
(--statistics-only, --data-only, --no-schema) didn't dump roles and
tablespaces. With the current patch, this is no longer true, and that
doesn't seem to be an intended change, at least it's not explained in
the commit message. The removed condition that causes this was already
mentioned previously, but without explicitly stating that this results
in a behavior change.
On Thu, Feb 5, 2026 at 2:33 AM Nathan Bossart <nathandbossart@gmail.com> 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.
Attachments:
v3-0001-pg_dumpall-error-out-conflict-options.patchtext/x-patch; charset=US-ASCII; name=v3-0001-pg_dumpall-error-out-conflict-options.patchDownload+182-38
The attached patch also addresses the points mentioned by Zsolt Parragi.
old:
- if (!data_only && !statistics_only && !no_schema)
new:
+ shouldDumpTablespaces = !roles_only && !no_tablespaces && !data_only
&& !schema_only && !statistics_only;
+ shouldDumpRoles = !tablespaces_only && !data_only && !schema_only &&
!statistics_only;
This is still a user visible change: no_schema -> schema_only
And I don't think this change is good, roles and tablespaces are part
of the schema, without them, DDL statements later can fail.
The variables also should use under_score names, not camelCase.
And these two produce empty dumps, maybe they could result in errors instead:
pg_dumpall --globals-only --schema-only
pg_dumpall --globals-only --data-only
These produce an empty dump with a pg_dump error message, as pg_dump
blocks them, so they could use early errors:
pg_dumpall --data-only --statistics-only
pg_dumpall --schema-only --statistics-only
pg_dumpall --data-only --schema-only
I also wonder if it would be better to use a simple static array with
a helper struct to describe the blocked pairs, iterating it with a
simple for loop and generating error messages dynamically, instead of
manual copy-paste and editing. This if list is already getting quite
long, and it doesn't contain all combinations that should be blocked
yet.
E.g.
incompatible_options[] = { { &globals_only, &roles_only,
"-g/--globals-only", "-r/--roles-only" }, ... }
On Fri, Feb 6, 2026 at 4:35 AM Zsolt Parragi <zsolt.parragi@percona.com> wrote:
The attached patch also addresses the points mentioned by Zsolt Parragi.
old:
- if (!data_only && !statistics_only && !no_schema)
new:
+ shouldDumpTablespaces = !roles_only && !no_tablespaces && !data_only && !schema_only && !statistics_only; + shouldDumpRoles = !tablespaces_only && !data_only && !schema_only && !statistics_only;This is still a user visible change: no_schema -> schema_only
And I don't think this change is good, roles and tablespaces are part
of the schema, without them, DDL statements later can fail.
hi.
I believe "schema" generally refers to object definitions, excluding
things like roles and tablespaces.
I tend to think that once "only" is specified, the "no" option meaning
is not applied,
thus I'm ok with
pg_dumpall --globals-only --no-schema
is equivalent to
pg_dumpall --globals-only
For all these pg_dumpall combination
--roles-only
--tablespaces-only
--statistics-only
--schema-only
--globals-only
--data-only
--statistics
The only allowed combination is --statistics --statistics-only.
since pg_dump also supports it, and these two option meanings do not contradict.
please check v4, it looks very neat, IMHO.
for example:
+ 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");
+
src/bin/pg_dump/t/001_basic.pl tests are well aligned with pg_dumpall.c code, so
it's quite easy to review.
Attachments:
v4-0001-pg_dumpall-error-out-conflict-options.patchtext/x-patch; charset=US-ASCII; name=v4-0001-pg_dumpall-error-out-conflict-options.patchDownload+198-57
Hello!
I believe "schema" generally refers to object definitions, excluding
things like roles and tablespaces.
Please see the attached tap test case. It works with the current
master branch, and fails with the patch. Basically my expectation is
that if we use dumpall --schema-only, we should be able to restore it
without errors, except for one error for the already existing
postgres/current user which it ignores.
I also found one more issue/behavior change: --no-schema --clean now
generates DROP statements (roles, tablespaces, databases), while
previously it didn't.
Attachments:
On Fri, Feb 6, 2026 at 5:36 PM Zsolt Parragi <zsolt.parragi@percona.com> wrote:
Please see the attached tap test case. It works with the current
master branch, and fails with the patch. Basically my expectation is
that if we use dumpall --schema-only, we should be able to restore it
without errors, except for one error for the already existing
postgres/current user which it ignores.I also found one more issue/behavior change: --no-schema --clean now
generates DROP statements (roles, tablespaces, databases), while
previously it didn't.
hi.
It would be better to simply reject the CONFLICT ONLY option and keep the rest
of the logic same as the HEAD. That way, we avoid any surprising behavior.
IMHO.
Please check attached v5.
Attachments:
v5-0001-pg_dumpall-error-out-conflict-options.patchtext/x-patch; charset=US-ASCII; name=v5-0001-pg_dumpall-error-out-conflict-options.patchDownload+172-28
It would be better to simply reject the CONFLICT ONLY option and keep the rest
of the logic same as the HEAD.
I agree, that's why I showed that test case that failed.
This version is definitely better, it's also more readable than the
previous version with the many ifs.
But it is again missing a few cases that should error out before
pg_dump, as currently it fails after pg_dump errors out:
--schema-only --no-schema
--data-only --no-data
--statistics-only --no-statistics
/* Make sure the user hasn't specified a mix of globals-only options */
- if (globals_only && roles_only)
+ if (globals_only &&
+ (roles_only || tablespaces_only || with_statistics ||
statistics_only || schema_only || data_only))
The comment above the checks also seems stale, as we have more checks
with the patch
On Mon, Feb 09, 2026 at 07:09:54PM +0000, Zsolt Parragi wrote:
But it is again missing a few cases that should error out before
pg_dump, as currently it fails after pg_dump errors out:--schema-only --no-schema
--data-only --no-data
--statistics-only --no-statistics
Is there a reason we need to duplicate these checks in pg_dumpall when they
are already handled by pg_dump?
--
nathan
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.
On Tue, Feb 10, 2026 at 3:47 AM Zsolt Parragi <zsolt.parragi@percona.com> 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.
Attachments:
v6-0001-pg_dumpall-error-out-conflict-options.patchtext/x-patch; charset=US-ASCII; name=v6-0001-pg_dumpall-error-out-conflict-options.patchDownload+206-32
Shouldn't these also use pg_log_error + pg_log_error_hint + exit_nicely?
And there's a commit message typo:
"there 3 combination should fail immediately"
there -> these
Otherwise it looks good to me.
On Wed, Feb 25, 2026 at 4:09 AM Zsolt Parragi <zsolt.parragi@percona.com> wrote:
Shouldn't these also use pg_log_error + pg_log_error_hint + exit_nicely?
Sure.
Otherwise it looks good to me.
While rebasing, I found that I missed the combination: --statistics
and --no-statistics.
Since pg_dump will error out on this combination, pg_dumpall should too.