Refactor handling of "-only" options in pg_dump, pg_restore

Started by jian he8 days ago5 messages
Jump to latest
#1jian he
jian.universality@gmail.com

Hi.

-------------------------------<<<<<<<
/* reject conflicting "-only" options */
if (data_only && schema_only)
pg_fatal("options %s and %s cannot be used together",
"-s/--schema-only", "-a/--data-only");
if (schema_only && statistics_only)
pg_fatal("options %s and %s cannot be used together",
"-s/--schema-only", "--statistics-only");
if (data_only && statistics_only)
pg_fatal("options %s and %s cannot be used together",
"-a/--data-only", "--statistics-only");

/* reject conflicting "-only" options */
if (data_only && with_statistics)
pg_fatal("options %s and %s cannot be used together",
"-a/--data-only", "--statistics");
if (schema_only && with_statistics)
pg_fatal("options %s and %s cannot be used together",
"-s/--schema-only", "--statistics");
-------------------------------<<<<<<<
The above is from src/bin/pg_dump/pg_dump.c, this is too much.

We can just use two IF statements:
if (data_only && (schema_only || with_statistics || statistics_only))
pg_fatal("options %s and %s cannot be used together",
"-a/--data-only",
schema_only ? "-s/--schema-only" :
with_statistics ? "--statistics" :
"--statistics-only");

if (schema_only && (with_statistics || statistics_only))
pg_fatal("options %s and %s cannot be used together",
"-s/--schema-only",
with_statistics ? "--statistics" :
"--statistics-only");

First "if (data_only && (schema_only" implies that the second IF check
won't have a combination
of `` if (schema_only && (data_only``.
Maybe we can use ELSE IF here.

We can do the same thing for pg_restore.c

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

Attachments:

v1-0001-pg_dump-pg_restore-refactor-only-option-error-check.patchtext/x-patch; charset=US-ASCII; name=v1-0001-pg_dump-pg_restore-refactor-only-option-error-check.patchDownload+26-37
#2Steven Niu
niushiji@gmail.com
In reply to: jian he (#1)
Re: Refactor handling of "-only" options in pg_dump, pg_restore

From: jian he <jian.universality@gmail.com>
Sent: Monday, March 02, 2026 12:57
To: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Refactor handling of "-only" options in pg_dump, pg_restore

Hi.

-------------------------------<<<<<<<
    /* reject conflicting "-only" options */
    if (data_only && schema_only)
        pg_fatal("options %s and %s cannot be used together",
                 "-s/--schema-only", "-a/--data-only");
    if (schema_only && statistics_only)
        pg_fatal("options %s and %s cannot be used together",
                 "-s/--schema-only", "--statistics-only");
    if (data_only && statistics_only)
        pg_fatal("options %s and %s cannot be used together",
                 "-a/--data-only", "--statistics-only");

    /* reject conflicting "-only" options */
    if (data_only && with_statistics)
        pg_fatal("options %s and %s cannot be used together",
                 "-a/--data-only", "--statistics");
    if (schema_only && with_statistics)
        pg_fatal("options %s and %s cannot be used together",
                 "-s/--schema-only", "--statistics");
-------------------------------<<<<<<<
The above is from src/bin/pg_dump/pg_dump.c, this is too much.

We can just use two IF statements:
    if (data_only && (schema_only || with_statistics || statistics_only))
        pg_fatal("options %s and %s cannot be used together",
                 "-a/--data-only",
                 schema_only ? "-s/--schema-only" :
                 with_statistics ? "--statistics" :
                 "--statistics-only");

    if (schema_only && (with_statistics || statistics_only))
        pg_fatal("options %s and %s cannot be used together",
                 "-s/--schema-only",
                 with_statistics ? "--statistics" :
                 "--statistics-only");

First "if (data_only && (schema_only" implies that the second IF check
won't have a combination
of `` if (schema_only && (data_only``.
Maybe we can use ELSE IF here.

We can do the same thing for pg_restore.c

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

Hi, jian,

Your change makes the code cleaner and easier to read. I like the idea.
Also I applied your patch locally and the make passed, "make check" in the src/bin/pg_dump also passed.

Regards,
Steven

#3Zsolt Parragi
zsolt.parragi@percona.com
In reply to: jian he (#1)
Re: Refactor handling of "-only" options in pg_dump, pg_restore

Hello

Simple patch and looks good, just one very minor about this comment:

/* reject conflicting "-only" options */

I would either remove the comment or the "-only" part, as that's no longer true.
(I see that it was there earlier in the later if, and it was similarly
incorrect. So this is not new with the patch, but still seems wrong)

#4jian he
jian.universality@gmail.com
In reply to: Zsolt Parragi (#3)
Re: Refactor handling of "-only" options in pg_dump, pg_restore

Hi.

Most of the code changes are very similar to
https://git.postgresql.org/cgit/postgresql.git/commit/?id=b2898baaf7e40a187de5b0134d53d944b38209cd
The change in pg_restore.c is more invasive, so I separated it from pg_dump.

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

Attachments:

v2-0002-pg_restore-Refactor-handling-of-conflicting-options.patchtext/x-patch; charset=US-ASCII; name=v2-0002-pg_restore-Refactor-handling-of-conflicting-options.patchDownload+54-101
v2-0001-pg_dump-Refactor-handling-of-conflicting-options.patchtext/x-patch; charset=US-ASCII; name=v2-0001-pg_dump-Refactor-handling-of-conflicting-options.patchDownload+27-42
#5Nathan Bossart
nathandbossart@gmail.com
In reply to: jian he (#4)
Re: Refactor handling of "-only" options in pg_dump, pg_restore

On Mon, Mar 09, 2026 at 04:15:42PM +0800, jian he wrote:

Most of the code changes are very similar to
https://git.postgresql.org/cgit/postgresql.git/commit/?id=b2898baaf7e40a187de5b0134d53d944b38209cd
The change in pg_restore.c is more invasive, so I separated it from pg_dump.

Committed, thanks.

--
nathan