pg_dumpall: Fix handling of conflicting options.

pg_dumpall is missing checks for some conflicting options,
including those passed through to pg_dump.  To fix, introduce a
new function that checks whether mutually exclusive options are
set, and use that in pg_dumpall.  A similar change could likely be
made for pg_dump and pg_restore, but that is left as a future
exercise.

This is arguably a bug fix, but since this might break existing
scripts, no back-patch for now.

Author: Jian He <jian.universality@gmail.com>
Co-authored-by: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Wang Peng <215722532@qq.com>
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/CACJufxFf5%3DwSv2MsuO8iZOvpLZQ1-meAMwhw7JX5gNvWo5PDug%40mail.gmail.com

src/bin/pg_dump/pg_dumpall.c

@@ -34,6 +34,7 @@
 #include "common/string.h"
 #include "connectdb.h"
 #include "dumputils.h"
+#include "fe_utils/option_utils.h"
 #include "fe_utils/string_utils.h"
 #include "filter.h"
 #include "getopt_long.h"
@@ -219,6 +220,7 @@ main(int argc, char *argv[])
 	bool		data_only = false;
 	bool		globals_only = false;
 	bool		roles_only = false;
+	bool		schema_only = false;
 	bool		tablespaces_only = false;
 	PGconn	   *conn;
 	int			encoding;
@@ -321,6 +323,7 @@ main(int argc, char *argv[])
 				break;
 
 			case 's':
+				schema_only = true;
 				appendPQExpBufferStr(pgdumpopts, " -s");
 				break;
 
@@ -418,45 +421,44 @@ main(int argc, char *argv[])
 		exit_nicely(1);
 	}
 
-	if (database_exclude_patterns.head != NULL &&
+	/* --exclude_database is incompatible with global *-only options */
-		(globals_only || roles_only || tablespaces_only))
+	check_mut_excl_opts(database_exclude_patterns.head, "--exclude-database",
-	{
+						globals_only, "-g/--globals-only",
-		pg_log_error("option %s cannot be used together with %s, %s, or %s",
+						roles_only, "-r/--roles-only",
-					 "--exclude-database",
+						tablespaces_only, "-t/--tablespaces-only");
-					 "-g/--globals-only", "-r/--roles-only", "-t/--tablespaces-only");
-		pg_log_error_hint("Try \"%s --help\" for more information.", progname);
-		exit_nicely(1);
-	}
 
-	/* Make sure the user hasn't specified a mix of globals-only options */
+	/* *-only options are incompatible with each other */
-	if (globals_only && roles_only)
+	check_mut_excl_opts(data_only, "-a/--data-only",
-	{
+						globals_only, "-g/--globals-only",
-		pg_log_error("options %s and %s cannot be used together",
+						roles_only, "-r/--roles-only",
-					 "-g/--globals-only", "-r/--roles-only");
+						schema_only, "-s/--schema-only",
-		pg_log_error_hint("Try \"%s --help\" for more information.", progname);
+						statistics_only, "--statistics-only",
-		exit_nicely(1);
+						tablespaces_only, "-t/--tablespaces-only");
-	}
 
-	if (globals_only && tablespaces_only)
+	/* --no-* and *-only for same thing are incompatible */
-	{
+	check_mut_excl_opts(data_only, "-a/--data-only",
-		pg_log_error("options %s and %s cannot be used together",
+						no_data, "--no-data");
-					 "-g/--globals-only", "-t/--tablespaces-only");
+	check_mut_excl_opts(schema_only, "-s/--schema-only",
-		pg_log_error_hint("Try \"%s --help\" for more information.", progname);
+						no_schema, "--no-schema");
-		exit_nicely(1);
+	check_mut_excl_opts(statistics_only, "--statistics-only",
-	}
+						no_statistics, "--no-statistics");
+
+	/* --statistics and --no-statistics are incompatible */
+	check_mut_excl_opts(with_statistics, "--statistics",
+						no_statistics, "--no-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",
+						roles_only, "-r/--roles-only",
+						schema_only, "-s/--schema-only",
+						tablespaces_only, "-t/--tablespaces-only");
 
 	if (if_exists && !output_clean)
 		pg_fatal("option %s requires option %s",
 				 "--if-exists", "-c/--clean");
 
-	if (roles_only && tablespaces_only)
-	{
-		pg_log_error("options %s and %s cannot be used together",
-					 "-r/--roles-only", "-t/--tablespaces-only");
-		pg_log_error_hint("Try \"%s --help\" for more information.", progname);
-		exit_nicely(1);
-	}
-
 	/* Get format for dump. */
 	archDumpFormat = parseDumpFormat(format_name);
 

src/bin/pg_dump/t/001_basic.pl

@@ -240,8 +240,38 @@ command_fails_like(
 # also fails for -r and -t, but it seems pointless to add more tests for those.
 command_fails_like(
 	[ 'pg_dumpall', '--exclude-database=foo', '--globals-only' ],
-	qr/\Qpg_dumpall: error: option --exclude-database cannot be used together with -g\/--globals-only\E/,
+	qr/\Qpg_dumpall: error: options --exclude-database and -g\/--globals-only cannot be used together\E/,
-	'pg_dumpall: option --exclude-database cannot be used together with -g/--globals-only'
+	'pg_dumpall: options --exclude-database and -g/--globals-only cannot be used together'
+);
+
+command_fails_like(
+	[ 'pg_dumpall', '-a', '--no-data' ],
+	qr/\Qpg_dumpall: error: options -a\/--data-only and --no-data cannot be used together\E/,
+	'pg_dumpall: options -a\/--data-only and --no-data cannot be used together'
+);
+
+command_fails_like(
+	[ 'pg_dumpall', '-s', '--no-schema' ],
+	qr/\Qpg_dumpall: error: options -s\/--schema-only and --no-schema cannot be used together\E/,
+	'pg_dumpall: options -s\/--schema-only and --no-schema cannot be used together'
+);
+
+command_fails_like(
+	[ 'pg_dumpall', '--statistics-only', '--no-statistics' ],
+	qr/\Qpg_dumpall: error: options --statistics-only and --no-statistics cannot be used together\E/,
+	'pg_dumpall: options --statistics-only and --no-statistics cannot be used together'
+);
+
+command_fails_like(
+	[ 'pg_dumpall', '--statistics', '--no-statistics' ],
+	qr/\Qpg_dumpall: error: options --statistics and --no-statistics cannot be used together\E/,
+	'pg_dumpall: options --statistics-only and --no-statistics cannot be used together'
+);
+
+command_fails_like(
+	[ 'pg_dumpall', '--statistics', '--tablespaces-only' ],
+	qr/\Qpg_dumpall: error: options --statistics and -t\/--tablespaces-only cannot be used together\E/,
+	'pg_dumpall: options --statistics and -t\/--tablespaces-only cannot be used together'
 );
 
 command_fails_like(

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 => {

src/bin/pg_dump/t/005_pg_dump_filterfile.pl

@@ -571,8 +571,8 @@ command_fails_like(
 		'--filter' => "$tempdir/inputfile.txt",
 		'--globals-only'
 	],
-	qr/\Qpg_dumpall: error: option --exclude-database cannot be used together with -g\/--globals-only\E/,
+	qr/\Qpg_dumpall: error: options --exclude-database and -g\/--globals-only cannot be used together\E/,
-	'pg_dumpall: option --exclude-database cannot be used together with -g/--globals-only'
+	'pg_dumpall: options --exclude-database and -g/--globals-only cannot be used together'
 );
 
 # Test invalid filter command

src/fe_utils/option_utils.c

@@ -109,3 +109,38 @@ parse_sync_method(const char *optarg, DataDirSyncMethod *sync_method)
 
 	return true;
 }
+
+/*
+ * Fail with appropriate error if 2 or more of the specified options are set.
+ * The first parameter is the number of arguments.  Following that is an
+ * arbitrary number of bool/string pairs.  The bool indicates whether the
+ * option is set, and the string is used for the error message.  Note that only
+ * the first pair of enabled options we find are reported.
+ *
+ * Don't call this directly.  Use the check_mut_excl_opts() macro in
+ * option_utils.h, which is the exact same except it doesn't take the first
+ * parameter (it discovers the number of arguments automagically).
+ */
+void
+check_mut_excl_opts_internal(int n,...)
+{
+	char	   *first = NULL;
+	va_list		args;
+
+	Assert(n % 2 == 0);
+
+	va_start(args, n);
+	for (int i = 0; i < n; i += 2)
+	{
+		bool		set = va_arg(args, int);
+		char	   *opt = va_arg(args, char *);
+
+		if (set && first)
+			pg_fatal("options %s and %s cannot be used together",
+					 first, opt);
+
+		if (set)
+			first = opt;
+	}
+	va_end(args);
+}

src/include/fe_utils/option_utils.h

@@ -24,5 +24,11 @@ extern bool option_parse_int(const char *optarg, const char *optname,
 							 int *result);
 extern bool parse_sync_method(const char *optarg,
 							  DataDirSyncMethod *sync_method);
+extern void check_mut_excl_opts_internal(int n,...);
+
+/* see comment for check_mut_excl_opts_internal() in option_utils.c for info */
+#define check_mut_excl_opts(set, opt, ...) \
+	check_mut_excl_opts_internal(VA_ARGS_NARGS(__VA_ARGS__) + 2, \
+								 set, opt, __VA_ARGS__)
 
 #endif							/* OPTION_UTILS_H */