pg_dumpall --roles-only interact with other options

Started by jian heabout 1 month ago27 messages
Jump to latest
#1jian he
jian.universality@gmail.com

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?

--
jian
https://www.enterprisedb.com/

#2Nathan Bossart
nathandbossart@gmail.com
In reply to: jian he (#1)
Re: pg_dumpall --roles-only interact with other options

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

#3jian he
jian.universality@gmail.com
In reply to: Nathan Bossart (#2)
Re: pg_dumpall --roles-only interact with other options

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."

--
jian
https://www.enterprisedb.com/

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
#4Nathan Bossart
nathandbossart@gmail.com
In reply to: jian he (#3)
Re: pg_dumpall --roles-only interact with other options

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

#5wangpeng
215722532@qq.com
In reply to: jian he (#3)
Re: pg_dumpall --roles-only interact with other options

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 --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."

--
jian
https://www.enterprisedb.com/

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

#6jian he
jian.universality@gmail.com
In reply to: wangpeng (#5)
Re: pg_dumpall --roles-only interact with other options

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

--
jian
https://www.enterprisedb.com/

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
#7Nathan Bossart
nathandbossart@gmail.com
In reply to: jian he (#6)
Re: pg_dumpall --roles-only interact with other options

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

#8Zsolt Parragi
zsolt.parragi@percona.com
In reply to: jian he (#6)
Re: pg_dumpall --roles-only interact with other options

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.

#9jian he
jian.universality@gmail.com
In reply to: Nathan Bossart (#7)
Re: pg_dumpall --roles-only interact with other options

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.

--
jian
https://www.enterprisedb.com/

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
#10Zsolt Parragi
zsolt.parragi@percona.com
In reply to: jian he (#9)
Re: pg_dumpall --roles-only interact with other options

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" }, ... }

#11jian he
jian.universality@gmail.com
In reply to: Zsolt Parragi (#10)
Re: pg_dumpall --roles-only interact with other options

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.

--
jian
https://www.enterprisedb.com/

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
#12Zsolt Parragi
zsolt.parragi@percona.com
In reply to: jian he (#11)
Re: pg_dumpall --roles-only interact with other options

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:

007_schema_only_roles.plapplication/octet-stream; name=007_schema_only_roles.plDownload
#13jian he
jian.universality@gmail.com
In reply to: Zsolt Parragi (#12)
Re: pg_dumpall --roles-only interact with other options

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.

--
jian
https://www.enterprisedb.com/

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
#14Zsolt Parragi
zsolt.parragi@percona.com
In reply to: jian he (#13)
Re: pg_dumpall --roles-only interact with other options

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

#15Nathan Bossart
nathandbossart@gmail.com
In reply to: Zsolt Parragi (#14)
Re: pg_dumpall --roles-only interact with other options

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

#16Zsolt Parragi
zsolt.parragi@percona.com
In reply to: Nathan Bossart (#15)
Re: pg_dumpall --roles-only interact with other options

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.

#17jian he
jian.universality@gmail.com
In reply to: Zsolt Parragi (#16)
Re: pg_dumpall --roles-only interact with other options

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.

--
jian
https://www.enterprisedb.com/

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
#18Zsolt Parragi
zsolt.parragi@percona.com
In reply to: jian he (#17)
Re: pg_dumpall --roles-only interact with other options

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.

#19jian he
jian.universality@gmail.com
In reply to: Zsolt Parragi (#18)
Re: pg_dumpall --roles-only interact with other options

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.

--
jian
https://www.enterprisedb.com/

Attachments:

v7-0001-pg_dumpall-error-out-conflict-options.patchtext/x-patch; charset=US-ASCII; name=v7-0001-pg_dumpall-error-out-conflict-options.patchDownload+235-33
#20Zsolt Parragi
zsolt.parragi@percona.com
In reply to: jian he (#19)
Re: pg_dumpall --roles-only interact with other options

New version looks good!

#21Nathan Bossart
nathandbossart@gmail.com
In reply to: Zsolt Parragi (#20)
#22jian he
jian.universality@gmail.com
In reply to: Nathan Bossart (#21)
#23Chao Li
li.evan.chao@gmail.com
In reply to: Nathan Bossart (#21)
#24Zsolt Parragi
zsolt.parragi@percona.com
In reply to: Chao Li (#23)
#25Nathan Bossart
nathandbossart@gmail.com
In reply to: Chao Li (#23)
#26Nathan Bossart
nathandbossart@gmail.com
In reply to: Nathan Bossart (#25)
#27Nathan Bossart
nathandbossart@gmail.com
In reply to: Nathan Bossart (#26)