From 7c8280eeb5872f5c2663b562a9c6fcf8ec8a4b82 Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Mon, 9 Mar 2026 11:37:46 -0500 Subject: [PATCH] pg_{dump,restore}: Refactor handling of conflicting options. This commit makes use of the function added by commit b2898baaf7 for these applications' handling of conflicting options. It doesn't fix any bugs, but it does trim several lines of code. Author: Jian He Reviewed-by: Steven Niu Reviewed-by: Zsolt Parragi Discussion: https://postgr.es/m/CACJufxHDYn%2B3-2jR_kwYB0U7UrNP%2B0EPvAWzBBD5EfUzzr1uiw%40mail.gmail.com --- src/bin/pg_dump/pg_dump.c | 61 +++++-------- src/bin/pg_dump/pg_dumpall.c | 2 +- src/bin/pg_dump/pg_restore.c | 136 ++++++++++------------------ src/bin/pg_dump/t/001_basic.pl | 26 +++--- src/bin/pg_dump/t/002_pg_dump.pl | 4 +- src/bin/pg_dump/t/007_pg_dumpall.pl | 8 +- 6 files changed, 91 insertions(+), 146 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 8bde1b382de..137161aa5e0 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -826,52 +826,39 @@ main(int argc, char **argv) if (dopt.column_inserts && dopt.dump_inserts == 0) dopt.dump_inserts = DUMP_DEFAULT_ROWS_PER_INSERT; - /* 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"); + /* *-only options are incompatible with each other */ + check_mut_excl_opts(data_only, "-a/--data-only", + schema_only, "-s/--schema-only", + statistics_only, "--statistics-only"); - /* reject conflicting "-only" and "no-" options */ - if (data_only && no_data) - pg_fatal("options %s and %s cannot be used together", - "-a/--data-only", "--no-data"); - if (schema_only && no_schema) - pg_fatal("options %s and %s cannot be used together", - "-s/--schema-only", "--no-schema"); - if (statistics_only && no_statistics) - pg_fatal("options %s and %s cannot be used together", - "--statistics-only", "--no-statistics"); + /* --no-* and *-only for same thing are incompatible */ + check_mut_excl_opts(data_only, "-a/--data-only", + no_data, "--no-data"); + check_mut_excl_opts(schema_only, "-s/--schema-only", + no_schema, "--no-schema"); + check_mut_excl_opts(statistics_only, "--statistics-only", + no_statistics, "--no-statistics"); - /* reject conflicting "no-" options */ - if (with_statistics && no_statistics) - pg_fatal("options %s and %s cannot be used together", - "--statistics", "--no-statistics"); + /* --statistics and --no-statistics are incompatible */ + check_mut_excl_opts(with_statistics, "--statistics", + no_statistics, "--no-statistics"); - /* 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"); + /* --statistics is incompatible with *-only (except --statistics-only) */ + check_mut_excl_opts(with_statistics, "--statistics", + data_only, "-a/--data-only", + schema_only, "-s/--schema-only"); - if (schema_only && foreign_servers_include_patterns.head != NULL) - pg_fatal("options %s and %s cannot be used together", - "-s/--schema-only", "--include-foreign-data"); + /* --include-foreign-data is incompatible with --schema-only */ + check_mut_excl_opts(foreign_servers_include_patterns.head, "--include-foreign-data", + schema_only, "-s/--schema-only"); if (numWorkers > 1 && foreign_servers_include_patterns.head != NULL) pg_fatal("option %s is not supported with parallel backup", "--include-foreign-data"); - if (data_only && dopt.outputClean) - pg_fatal("options %s and %s cannot be used together", - "-c/--clean", "-a/--data-only"); + /* --clean is incompatible with --data-only */ + check_mut_excl_opts(dopt.outputClean, "-c/--clean", + data_only, "-a/--data-only"); if (dopt.if_exists && !dopt.outputClean) pg_fatal("option %s requires option %s", diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c index 4ded9020952..b29eaa81938 100644 --- a/src/bin/pg_dump/pg_dumpall.c +++ b/src/bin/pg_dump/pg_dumpall.c @@ -421,7 +421,7 @@ main(int argc, char *argv[]) exit_nicely(1); } - /* --exclude_database is incompatible with global *-only options */ + /* --exclude-database is incompatible with global *-only options */ check_mut_excl_opts(database_exclude_patterns.head, "--exclude-database", globals_only, "-g/--globals-only", roles_only, "-r/--roles-only", diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c index 752d859e264..fb44c0cfdfe 100644 --- a/src/bin/pg_dump/pg_restore.c +++ b/src/bin/pg_dump/pg_restore.c @@ -385,31 +385,20 @@ main(int argc, char **argv) if (!opts->cparams.dbname && !opts->filename && !opts->tocSummary) pg_fatal("one of -d/--dbname and -f/--file must be specified"); - if (db_exclude_patterns.head != NULL && globals_only) - { - pg_log_error("option %s cannot be used together with %s", - "--exclude-database", "-g/--globals-only"); - pg_log_error_hint("Try \"%s --help\" for more information.", progname); - exit_nicely(1); - } + /* --exclude-database and --globals-only are incompatible */ + check_mut_excl_opts(db_exclude_patterns.head, "--exclude-database", + globals_only, "-g/--globals-only"); /* Should get at most one of -d and -f, else user is confused */ + check_mut_excl_opts(opts->cparams.dbname, "-d/--dbname", + opts->filename, "-f/--file"); + + /* --dbname and --restrict-key are incompatible */ + check_mut_excl_opts(opts->cparams.dbname, "-d/--dbname", + opts->restrict_key, "--restrict-key"); + if (opts->cparams.dbname) - { - if (opts->filename) - { - pg_log_error("options %s and %s cannot be used together", - "-d/--dbname", "-f/--file"); - pg_log_error_hint("Try \"%s --help\" for more information.", progname); - exit_nicely(1); - } - - if (opts->restrict_key) - pg_fatal("options %s and %s cannot be used together", - "-d/--dbname", "--restrict-key"); - opts->useDB = 1; - } else { /* @@ -423,85 +412,54 @@ main(int argc, char **argv) pg_fatal("invalid restrict key"); } - /* 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"); + /* *-only options are incompatible with each other */ + check_mut_excl_opts(data_only, "-a/--data-only", + globals_only, "-g/--globals-only", + schema_only, "-s/--schema-only", + statistics_only, "--statistics-only"); - /* reject conflicting "-only" and "no-" options */ - if (data_only && no_data) - pg_fatal("options %s and %s cannot be used together", - "-a/--data-only", "--no-data"); - if (schema_only && no_schema) - pg_fatal("options %s and %s cannot be used together", - "-s/--schema-only", "--no-schema"); - if (statistics_only && no_statistics) - pg_fatal("options %s and %s cannot be used together", - "--statistics-only", "--no-statistics"); + /* --no-* and *-only for same thing are incompatible */ + check_mut_excl_opts(data_only, "-a/--data-only", + no_data, "--no-data"); + check_mut_excl_opts(globals_only, "-g/--globals-only", + no_globals, "--no-globals"); + check_mut_excl_opts(schema_only, "-s/--schema-only", + no_schema, "--no-schema"); + check_mut_excl_opts(statistics_only, "--statistics-only", + no_statistics, "--no-statistics"); - /* reject conflicting "no-" options */ - if (with_statistics && no_statistics) - pg_fatal("options %s and %s cannot be used together", - "--statistics", "--no-statistics"); + /* --statistics and --no-statistics are incompatible */ + check_mut_excl_opts(with_statistics, "--statistics", + no_statistics, "--no-statistics"); - /* 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"); + /* --statistics is incompatible with *-only (except --statistics-only) */ + check_mut_excl_opts(with_statistics, "--statistics", + data_only, "-a/--data-only", + globals_only, "-g/--globals-only", + schema_only, "-s/--schema-only"); - if (data_only && opts->dropSchema) - pg_fatal("options %s and %s cannot be used together", - "-c/--clean", "-a/--data-only"); + /* --clean and --data-only are incompatible */ + check_mut_excl_opts(opts->dropSchema, "-c/--clean", + data_only, "-a/--data-only"); - if (opts->single_txn && opts->txn_size > 0) - pg_fatal("options %s and %s cannot be used together", - "-1/--single-transaction", "--transaction-size"); + /* + * --globals-only, --single-transaction, and --transaction-size are + * incompatible. + */ + check_mut_excl_opts(globals_only, "-g/--globals-only", + opts->single_txn, "-1/--single-transaction", + opts->txn_size, "--transaction-size"); - if (opts->single_txn && globals_only) - pg_fatal("options %s and %s cannot be used together when restoring an archive created by pg_dumpall", - "--single-transaction", "-g/--globals-only"); - - if (opts->txn_size && globals_only) - pg_fatal("options %s and %s cannot be used together when restoring an archive created by pg_dumpall", - "--transaction-size", "-g/--globals-only"); - - if (opts->exit_on_error && globals_only) - pg_fatal("options %s and %s cannot be used together when restoring an archive created by pg_dumpall", - "--exit-on-error", "-g/--globals-only"); - - if (data_only && globals_only) - pg_fatal("options %s and %s cannot be used together", - "-a/--data-only", "-g/--globals-only"); - if (schema_only && globals_only) - pg_fatal("options %s and %s cannot be used together", - "-s/--schema-only", "-g/--globals-only"); - if (statistics_only && globals_only) - pg_fatal("options %s and %s cannot be used together", - "--statistics-only", "-g/--globals-only"); - if (with_statistics && globals_only) - pg_fatal("options %s and %s cannot be used together", - "--statistics", "-g/--globals-only"); - - if (no_globals && globals_only) - pg_fatal("options %s and %s cannot be used together", - "--no-globals", "-g/--globals-only"); + /* --exit-on-error and --globals-only are incompatible */ + check_mut_excl_opts(opts->exit_on_error, "--exit-on-error", + globals_only, "-g/--globals-only"); /* * -C is not compatible with -1, because we can't create a database inside * a transaction block. */ - if (opts->createDB && opts->single_txn) - pg_fatal("options %s and %s cannot be used together", - "-C/--create", "-1/--single-transaction"); + check_mut_excl_opts(opts->createDB, "-C/--create", + opts->single_txn, "-1/--single-transaction"); /* Can't do single-txn mode with multiple connections */ if (opts->single_txn && numWorkers > 1) diff --git a/src/bin/pg_dump/t/001_basic.pl b/src/bin/pg_dump/t/001_basic.pl index 67131a674f4..2f5eb48e7b8 100644 --- a/src/bin/pg_dump/t/001_basic.pl +++ b/src/bin/pg_dump/t/001_basic.pl @@ -46,8 +46,8 @@ command_fails_like( command_fails_like( [ 'pg_dump', '-s', '-a' ], - qr/\Qpg_dump: error: options -s\/--schema-only and -a\/--data-only cannot be used together\E/, - 'pg_dump: options -s/--schema-only and -a/--data-only cannot be used together' + qr/\Qpg_dump: error: options -a\/--data-only and -s\/--schema-only cannot be used together\E/, + 'pg_dump: options -a/--data-only and -s/--schema-only cannot be used together' ); command_fails_like( @@ -64,8 +64,8 @@ command_fails_like( command_fails_like( [ 'pg_dump', '-s', '--include-foreign-data=xxx' ], - qr/\Qpg_dump: error: options -s\/--schema-only and --include-foreign-data cannot be used together\E/, - 'pg_dump: options -s/--schema-only and --include-foreign-data cannot be used together' + qr/\Qpg_dump: error: options --include-foreign-data and -s\/--schema-only cannot be used together\E/, + 'pg_dump: options --include-foreign-data and -s/--schema-only cannot be used together' ); command_fails_like( @@ -87,8 +87,8 @@ command_fails_like( command_fails_like( [ 'pg_restore', '-s', '-a', '-f -' ], - qr/\Qpg_restore: error: options -s\/--schema-only and -a\/--data-only cannot be used together\E/, - 'pg_restore: options -s/--schema-only and -a/--data-only cannot be used together' + qr/\Qpg_restore: error: options -a\/--data-only and -s\/--schema-only cannot be used together\E/, + 'pg_restore: options -a/--data-only and -s/--schema-only cannot be used together' ); command_fails_like( @@ -300,8 +300,8 @@ command_fails_like( command_fails_like( [ 'pg_restore', '--exclude-database=foo', '--globals-only', '-d', 'xxx' ], - qr/\Qpg_restore: error: option --exclude-database cannot be used together with -g\/--globals-only\E/, - 'pg_restore: option --exclude-database cannot be used together with -g/--globals-only' + qr/\Qpg_restore: error: options --exclude-database and -g\/--globals-only cannot be used together\E/, + 'pg_restore: options --exclude-database and -g/--globals-only cannot be used together' ); command_fails_like( @@ -312,14 +312,14 @@ command_fails_like( command_fails_like( [ 'pg_restore', '--schema-only', '--globals-only', '-d', 'xxx' ], - qr/\Qpg_restore: error: options -s\/--schema-only and -g\/--globals-only cannot be used together\E/, - 'pg_restore: error: options -s/--schema-only and -g/--globals-only cannot be used together' + qr/\Qpg_restore: error: options -g\/--globals-only and -s\/--schema-only cannot be used together\E/, + 'pg_restore: error: options -g/--globals-only and -s/--schema-only cannot be used together' ); command_fails_like( [ 'pg_restore', '--statistics-only', '--globals-only', '-d', 'xxx' ], - qr/\Qpg_restore: error: options --statistics-only and -g\/--globals-only cannot be used together\E/, - 'pg_restore: error: options --statistics-only and -g/--globals-only cannot be used together' + qr/\Qpg_restore: error: options -g\/--globals-only and --statistics-only cannot be used together\E/, + 'pg_restore: error: options -g/--globals-only and --statistics-only cannot be used together' ); command_fails_like( @@ -339,6 +339,6 @@ command_fails_like( 'pg_restore', '--globals-only', '--no-globals', '-d', 'xxx', 'dumpdir' ], - qr/\Qpg_restore: error: options --no-globals and -g\/--globals-only cannot be used together\E/, + qr/\Qpg_restore: error: options -g\/--globals-only and --no-globals cannot be used together\E/, 'options --no-globals and --globals-only cannot be used together'); done_testing(); diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index e7cc998cfba..6d1d38128fc 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -5077,8 +5077,8 @@ command_fails_like( '--schema-only', '--statistics', ], - qr/\Qpg_dump: error: options -s\/--schema-only and --statistics cannot be used together\E/, - 'cannot use --schema-only and --statistics together'); + qr/\Qpg_dump: error: options --statistics and -s\/--schema-only cannot be used together\E/, + 'cannot use --statistics and --schema-only together'); command_fails_like( [ diff --git a/src/bin/pg_dump/t/007_pg_dumpall.pl b/src/bin/pg_dump/t/007_pg_dumpall.pl index c16c27d7387..22f11a13a9a 100644 --- a/src/bin/pg_dump/t/007_pg_dumpall.pl +++ b/src/bin/pg_dump/t/007_pg_dumpall.pl @@ -520,7 +520,7 @@ $node->command_fails_like( '--schema-only', '--file' => "$tempdir/error_test.sql", ], - qr/\Qpg_restore: error: options -s\/--schema-only and -g\/--globals-only cannot be used together\E/, + qr/\Qpg_restore: error: options -g\/--globals-only and -s\/--schema-only cannot be used together\E/, 'When --globals-only and --schema-only are used together'); # report an error when --globals-only and --statistics-only are used together @@ -533,7 +533,7 @@ $node->command_fails_like( '--statistics-only', '--file' => "$tempdir/error_test.sql", ], - qr/\Qpg_restore: error: options --statistics-only and -g\/--globals-only cannot be used together\E/, + qr/\Qpg_restore: error: options -g\/--globals-only and --statistics-only cannot be used together\E/, 'When --globals-only and --statistics-only are used together'); # report an error when --globals-only and --statistics are used together @@ -572,7 +572,7 @@ $node->command_fails_like( '--single-transaction', '--file' => "$tempdir/error_test.sql", ], - qr/\Qpg_restore: error: options --single-transaction and -g\/--globals-only cannot be used together\E/, + qr/\Qpg_restore: error: options -g\/--globals-only and -1\/--single-transaction cannot be used together\E/, 'When --globals-only and --single-transaction are used together'); # report an error when --globals-only and --transaction-size are used together @@ -585,7 +585,7 @@ $node->command_fails_like( '--transaction-size' => '100', '--file' => "$tempdir/error_test.sql", ], - qr/\Qpg_restore: error: options --transaction-size and -g\/--globals-only cannot be used together\E/, + qr/\Qpg_restore: error: options -g\/--globals-only and --transaction-size cannot be used together\E/, 'When --globals-only and --transaction-size are used together'); # verify map.dat preamble exists