reindexdb: Allow specifying objects to process in all databases.

Presently, reindexdb's --table, --schema, --index, and --system
options cannot be used together with --all, i.e., you cannot
specify objects to process in all databases.  This commit removes
this unnecessary restriction.  Furthermore, it removes the
restriction that --system cannot be used with --table, --schema,
and --index.  There is no such restriction for the latter options,
and there is no technical reason to disallow these combinations.

Reviewed-by: Kyotaro Horiguchi, Dean Rasheed
Discussion: https://postgr.es/m/20230628232402.GA1954626%40nathanxps13

Author: nathan-bossart

doc/src/sgml/ref/reindexdb.sgml

@@ -55,30 +55,22 @@ PostgreSQL documentation
     </arg>
    </arg>
 
-   <arg choice="opt"><replaceable>dbname</replaceable></arg>
-  </cmdsynopsis>
-
-  <cmdsynopsis>
-   <command>reindexdb</command>
-   <arg rep="repeat"><replaceable>connection-option</replaceable></arg>
-   <arg rep="repeat"><replaceable>option</replaceable></arg>
-
-   <group choice="plain">
-    <arg choice="plain"><option>-a</option></arg>
-    <arg choice="plain"><option>--all</option></arg>
-   </group>
-  </cmdsynopsis>
-
-  <cmdsynopsis>
-   <command>reindexdb</command>
-   <arg rep="repeat"><replaceable>connection-option</replaceable></arg>
-   <arg rep="repeat"><replaceable>option</replaceable></arg>
+   <arg choice="plain">
+    <arg choice="opt">
+     <group choice="plain">
+      <arg choice="plain"><option>-s</option></arg>
+      <arg choice="plain"><option>--system</option></arg>
+     </group>
+    </arg>
+   </arg>
 
-   <group choice="plain">
-    <arg choice="plain"><option>-s</option></arg>
-    <arg choice="plain"><option>--system</option></arg>
-   </group>
-   <arg choice="opt"><replaceable>dbname</replaceable></arg>
+   <arg choice="opt">
+    <group choice="plain">
+     <arg choice="plain"><replaceable>dbname</replaceable></arg>
+     <arg choice="plain"><option>-a</option></arg>
+     <arg choice="plain"><option>--all</option></arg>
+    </group>
+   </arg>
   </cmdsynopsis>
  </refsynopsisdiv>
 

src/bin/scripts/reindexdb.c

@@ -46,7 +46,10 @@ static void reindex_one_database(ConnParams *cparams, ReindexType type,
 static void reindex_all_databases(ConnParams *cparams,
 								  const char *progname, bool echo,
 								  bool quiet, bool verbose, bool concurrently,
-								  int concurrentCons, const char *tablespace);
+								  int concurrentCons, const char *tablespace,
+								  bool syscatalog, SimpleStringList *schemas,
+								  SimpleStringList *tables,
+								  SimpleStringList *indexes);
 static void run_reindex_command(PGconn *conn, ReindexType type,
 								const char *name, bool echo, bool verbose,
 								bool concurrently, bool async,
@@ -203,62 +206,33 @@ main(int argc, char *argv[])
 
 	setup_cancel_handler(NULL);
 
+	if (concurrentCons > 1)
+	{
+		/*
+		 * Index-level REINDEX is not supported with multiple jobs as we
+		 * cannot control the concurrent processing of multiple indexes
+		 * depending on the same relation.
+		 */
+		if (indexes.head != NULL)
+			pg_fatal("cannot use multiple jobs to reindex indexes");
+
+		if (syscatalog)
+			pg_fatal("cannot use multiple jobs to reindex system catalogs");
+	}
+
 	if (alldb)
 	{
 		if (dbname)
 			pg_fatal("cannot reindex all databases and a specific one at the same time");
-		if (syscatalog)
-			pg_fatal("cannot reindex all databases and system catalogs at the same time");
-		if (schemas.head != NULL)
-			pg_fatal("cannot reindex specific schema(s) in all databases");
-		if (tables.head != NULL)
-			pg_fatal("cannot reindex specific table(s) in all databases");
-		if (indexes.head != NULL)
-			pg_fatal("cannot reindex specific index(es) in all databases");
 
 		cparams.dbname = maintenance_db;
 
 		reindex_all_databases(&cparams, progname, echo, quiet, verbose,
-							  concurrently, concurrentCons, tablespace);
-	}
-	else if (syscatalog)
-	{
-		if (schemas.head != NULL)
-			pg_fatal("cannot reindex specific schema(s) and system catalogs at the same time");
-		if (tables.head != NULL)
-			pg_fatal("cannot reindex specific table(s) and system catalogs at the same time");
-		if (indexes.head != NULL)
-			pg_fatal("cannot reindex specific index(es) and system catalogs at the same time");
-
-		if (concurrentCons > 1)
-			pg_fatal("cannot use multiple jobs to reindex system catalogs");
-
-		if (dbname == NULL)
-		{
-			if (getenv("PGDATABASE"))
-				dbname = getenv("PGDATABASE");
-			else if (getenv("PGUSER"))
-				dbname = getenv("PGUSER");
-			else
-				dbname = get_user_name_or_exit(progname);
-		}
-
-		cparams.dbname = dbname;
-
-		reindex_one_database(&cparams, REINDEX_SYSTEM, NULL,
-							 progname, echo, verbose,
-							 concurrently, 1, tablespace);
+							  concurrently, concurrentCons, tablespace,
+							  syscatalog, &schemas, &tables, &indexes);
 	}
 	else
 	{
-		/*
-		 * Index-level REINDEX is not supported with multiple jobs as we
-		 * cannot control the concurrent processing of multiple indexes
-		 * depending on the same relation.
-		 */
-		if (concurrentCons > 1 && indexes.head != NULL)
-			pg_fatal("cannot use multiple jobs to reindex indexes");
-
 		if (dbname == NULL)
 		{
 			if (getenv("PGDATABASE"))
@@ -271,6 +245,11 @@ main(int argc, char *argv[])
 
 		cparams.dbname = dbname;
 
+		if (syscatalog)
+			reindex_one_database(&cparams, REINDEX_SYSTEM, NULL,
+								 progname, echo, verbose,
+								 concurrently, 1, tablespace);
+
 		if (schemas.head != NULL)
 			reindex_one_database(&cparams, REINDEX_SCHEMA, &schemas,
 								 progname, echo, verbose,
@@ -287,10 +266,11 @@ main(int argc, char *argv[])
 								 concurrently, concurrentCons, tablespace);
 
 		/*
-		 * reindex database only if neither index nor table nor schema is
-		 * specified
+		 * reindex database only if neither index nor table nor schema nor
+		 * system catalogs is specified
 		 */
-		if (indexes.head == NULL && tables.head == NULL && schemas.head == NULL)
+		if (!syscatalog && indexes.head == NULL &&
+			tables.head == NULL && schemas.head == NULL)
 			reindex_one_database(&cparams, REINDEX_DATABASE, NULL,
 								 progname, echo, verbose,
 								 concurrently, concurrentCons, tablespace);
@@ -711,7 +691,9 @@ static void
 reindex_all_databases(ConnParams *cparams,
 					  const char *progname, bool echo, bool quiet, bool verbose,
 					  bool concurrently, int concurrentCons,
-					  const char *tablespace)
+					  const char *tablespace, bool syscatalog,
+					  SimpleStringList *schemas, SimpleStringList *tables,
+					  SimpleStringList *indexes)
 {
 	PGconn	   *conn;
 	PGresult   *result;
@@ -735,9 +717,35 @@ reindex_all_databases(ConnParams *cparams,
 
 		cparams->override_dbname = dbname;
 
-		reindex_one_database(cparams, REINDEX_DATABASE, NULL,
-							 progname, echo, verbose, concurrently,
-							 concurrentCons, tablespace);
+		if (syscatalog)
+			reindex_one_database(cparams, REINDEX_SYSTEM, NULL,
+								 progname, echo, verbose,
+								 concurrently, 1, tablespace);
+
+		if (schemas->head != NULL)
+			reindex_one_database(cparams, REINDEX_SCHEMA, schemas,
+								 progname, echo, verbose,
+								 concurrently, concurrentCons, tablespace);
+
+		if (indexes->head != NULL)
+			reindex_one_database(cparams, REINDEX_INDEX, indexes,
+								 progname, echo, verbose,
+								 concurrently, 1, tablespace);
+
+		if (tables->head != NULL)
+			reindex_one_database(cparams, REINDEX_TABLE, tables,
+								 progname, echo, verbose,
+								 concurrently, concurrentCons, tablespace);
+
+		/*
+		 * reindex database only if neither index nor table nor schema nor
+		 * system catalogs is specified
+		 */
+		if (!syscatalog && indexes->head == NULL &&
+			tables->head == NULL && schemas->head == NULL)
+			reindex_one_database(cparams, REINDEX_DATABASE, NULL,
+								 progname, echo, verbose,
+								 concurrently, concurrentCons, tablespace);
 	}
 
 	PQclear(result);

src/bin/scripts/t/090_reindexdb.pl

@@ -262,4 +262,18 @@
 	[ 'reindexdb', '-j', '2', '--concurrently', '-d', 'postgres' ],
 	'parallel reindexdb on database, concurrently');
 
+# combinations of objects
+$node->issues_sql_like(
+	[ 'reindexdb', '-s', '-t', 'test1', 'postgres' ],
+	qr/statement:\ REINDEX SYSTEM postgres;/,
+	'specify both --system and --table');
+$node->issues_sql_like(
+	[ 'reindexdb', '-s', '-i', 'test1x', 'postgres' ],
+	qr/statement:\ REINDEX INDEX public.test1x;/,
+	'specify both --system and --index');
+$node->issues_sql_like(
+	[ 'reindexdb', '-s', '-S', 'pg_catalog', 'postgres' ],
+	qr/statement:\ REINDEX SCHEMA pg_catalog;/,
+	'specify both --system and --schema');
+
 done_testing();

src/bin/scripts/t/091_reindexdb_all.pl

@@ -13,10 +13,30 @@
 
 $ENV{PGOPTIONS} = '--client-min-messages=WARNING';
 
+$node->safe_psql('postgres',
+	'CREATE TABLE test1 (a int); CREATE INDEX test1x ON test1 (a);');
+$node->safe_psql('template1',
+	'CREATE TABLE test1 (a int); CREATE INDEX test1x ON test1 (a);');
 $node->issues_sql_like(
 	[ 'reindexdb', '-a' ],
 	qr/statement: REINDEX.*statement: REINDEX/s,
 	'reindex all databases');
+$node->issues_sql_like(
+	[ 'reindexdb', '-a', '-s' ],
+	qr/statement: REINDEX SYSTEM postgres/s,
+	'reindex system catalogs in all databases');
+$node->issues_sql_like(
+	[ 'reindexdb', '-a', '-S', 'public' ],
+	qr/statement: REINDEX SCHEMA public/s,
+	'reindex schema in all databases');
+$node->issues_sql_like(
+	[ 'reindexdb', '-a', '-i', 'test1x' ],
+	qr/statement: REINDEX INDEX public\.test1x/s,
+	'reindex index in all databases');
+$node->issues_sql_like(
+	[ 'reindexdb', '-a', '-t', 'test1' ],
+	qr/statement: REINDEX TABLE public\.test1/s,
+	'reindex table in all databases');
 
 $node->safe_psql(
 	'postgres', q(