| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
| Cc: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, jian he <jian(dot)universality(at)gmail(dot)com>, wangpeng <215722532(at)qq(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: pg_dumpall --roles-only interact with other options |
| Date: | 2026-03-03 02:46:51 |
| Message-ID: | 2FAB1AE3-7812-4603-A314-E2D3189AA5C8@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Mar 3, 2026, at 06:41, Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
>
> 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
> <v8-0001-pg_dumpall-error-out-conflict-options.patch>
Yeah, the new helper function makes the code much cleaner, good job.
A few comments on v8:
1 - dumputils.c
```
+void
+CheckMutuallyExclusiveOpts(int n,...)
+{
+ char *first = NULL;
+ va_list args;
+
+ va_start(args, n);
+ for (int i = 0; i < n; i += 2)
```
I think we can Assert(n % 2 == 0).
If a future code author mistakenly forgets an option name, the compiler won’t detect that, and runtime will only show “null” for the option name.
I tried to delete the last option name from the first CheckMutuallyExclusiveOpts call, then I got:
```
% pg_dumpall -a --no-data
pg_dumpall: error: options -a/--data-only and (null) cannot be used together
```
2 - dumputils.c
```
+ pg_fatal("options %s and %s cannot be used together",
+ first, opt);
```
The current code also shows a hint upon the error, do we want to retain that?
```
pg_log_error_hint("Try \"%s --help\" for more information.", progname);
```
3 - 001_basic.pl
```
+ 'pg_dumpall: error: options /-s\/--schema-only and --statistics-only cannot be used together'
```
“/“ before “-s” seems not needed.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Thomas Munro | 2026-03-03 02:55:04 | Exploring pass-by-value for small Bitmapset sets |
| Previous Message | Lukas Fittl | 2026-03-03 02:39:30 | Re: pg_buffercache: Add per-relation summary stats |