Le 11/04/2022 à 20:37, Nathan Bossart a écrit :
> On Fri, Apr 08, 2022 at 05:16:06PM +0200, Gilles Darold wrote:
>> Attached v7 of the patch that should pass cfbot.
> Thanks for the new patch!  Unfortunately, it looks like some recent changes
> have broken it again.
>
>> +enum trivalue schema_is_exclude = TRI_DEFAULT;
>> +
>> +/*
>> + * The kind of object filter to use. '0': none, 'n': schema, 't': table
>> + * these values correspond to the -n | -N and -t command line options.
>> + */
>> +char objfilter = '0';
> I think these should be combined into a single enum for simplicity and
> readability (e.g., OBJFILTER_NONE, OBJFILTER_INCLUDE_SCHEMA,
> OBJFILTER_EXCLUDE_SCHEMA, OBJFILTER_TABLE).
>
>>       * Instead, let the server decide whether a given relation can be
>>       * processed in which case the user will know about it.
>>       */
>> -    if (!tables_listed)
>> +    if (!objects_listed || objfilter == 'n')
>>      {
>>              appendPQExpBufferStr(&catalog_query, " WHERE c.relkind 
>> OPERATOR(pg_catalog.=) ANY (array["
>>                                                       
>> CppAsString2(RELKIND_RELATION) ", "
> I think this deserveѕ a comment.
>

Attached v8 of the patch that tries to address the remarks above, fixes
patch apply failure to master and replace calls to pg_log_error+exit
with pg_fatal.


.Thanks.

-- 
Gilles Darold
diff --git a/doc/src/sgml/ref/vacuumdb.sgml b/doc/src/sgml/ref/vacuumdb.sgml
index 956c0f01cb..0de001ef24 100644
--- a/doc/src/sgml/ref/vacuumdb.sgml
+++ b/doc/src/sgml/ref/vacuumdb.sgml
@@ -39,6 +39,40 @@ PostgreSQL documentation
    <arg choice="opt"><replaceable>dbname</replaceable></arg>
   </cmdsynopsis>
 
+  <cmdsynopsis>
+   <command>vacuumdb</command>
+   <arg rep="repeat"><replaceable>connection-option</replaceable></arg>
+   <arg rep="repeat"><replaceable>option</replaceable></arg>
+
+   <arg choice="plain" rep="repeat">
+    <arg choice="opt">
+     <group choice="plain">
+       <arg choice="plain">
+        <arg choice="opt">
+         <group choice="plain">
+          <arg choice="plain"><option>-n</option></arg>
+          <arg choice="plain"><option>--schema</option></arg>
+         </group>
+         <replaceable>schema</replaceable>
+        </arg>
+       </arg>
+
+       <arg choice="plain">
+        <arg choice="opt">
+         <group choice="plain">
+          <arg choice="plain"><option>-N</option></arg>
+          <arg choice="plain"><option>--exclude-schema</option></arg>
+         </group>
+         <replaceable>schema</replaceable>
+        </arg>
+       </arg>
+     </group>
+    </arg>
+   </arg>
+
+   <arg choice="opt"><replaceable>dbname</replaceable></arg>
+  </cmdsynopsis>
+
   <cmdsynopsis>
    <command>vacuumdb</command>
    <arg rep="repeat"><replaceable>connection-option</replaceable></arg>
@@ -244,6 +278,28 @@ PostgreSQL documentation
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><option>-n <replaceable class="parameter">schema</replaceable></option></term>
+      <term><option>--schema=<replaceable class="parameter">schema</replaceable></option></term>
+      <listitem>
+       <para>
+        Clean or analyze all tables in <replaceable class="parameter">schema</replaceable> only.
+        Multiple schemas can be vacuumed by writing multiple <option>-n</option> switches.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
+      <term><option>-N <replaceable class="parameter">schema</replaceable></option></term>
+      <term><option>--exclude-schema=<replaceable class="parameter">schema</replaceable></option></term>
+      <listitem>
+       <para>
+        Clean or analyze all tables NOT in <replaceable class="parameter">schema</replaceable>.
+        Multiple schemas can be excluded from the vacuum by writing multiple <option>-N</option> switches.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><option>--no-index-cleanup</option></term>
       <listitem>
@@ -619,6 +675,14 @@ PostgreSQL documentation
 <prompt>$ </prompt><userinput>vacuumdb --analyze --verbose --table='foo(bar)' xyzzy</userinput>
 </screen></para>
 
+   <para>
+    To clean all tables in the <literal>foo</literal> and <literal>bar</literal> schemas
+    only in a database named <literal>xyzzy</literal>:
+<screen>
+<prompt>$ </prompt><userinput>vacuumdb --schema='foo' --schema='bar' xyzzy</userinput>
+</screen></para>
+
+
  </refsect1>
 
  <refsect1>
diff --git a/src/bin/scripts/t/100_vacuumdb.pl b/src/bin/scripts/t/100_vacuumdb.pl
index 96a818a3c1..7bbfb97246 100644
--- a/src/bin/scripts/t/100_vacuumdb.pl
+++ b/src/bin/scripts/t/100_vacuumdb.pl
@@ -103,6 +103,8 @@ $node->safe_psql(
   CREATE TABLE funcidx (x int);
   INSERT INTO funcidx VALUES (0),(1),(2),(3);
   CREATE INDEX i0 ON funcidx ((f1(x)));
+  CREATE SCHEMA "Foo";
+  CREATE TABLE "Foo".bar(id int);
 |);
 $node->command_ok([qw|vacuumdb -Z --table="need""q(uot"(")x") postgres|],
 	'column list');
@@ -146,5 +148,15 @@ $node->issues_sql_like(
 	[ 'vacuumdb', '--min-xid-age', '2147483001', 'postgres' ],
 	qr/GREATEST.*relfrozenxid.*2147483001/,
 	'vacuumdb --table --min-xid-age');
+$node->issues_sql_like(
+	[ 'vacuumdb', '--schema', '"Foo"', 'postgres' ],
+	qr/VACUUM "Foo".bar/,
+	'vacuumdb --schema schema only');
+$node->command_fails(
+	[ 'vacuumdb',   '-n', 'pg_catalog', '-t', 'pg_class', 'postgres' ],
+	'cannot vacuum all tables in schema(s) and specific table(s) at the same time');
+$node->command_fails(
+	[ 'vacuumdb',   '-n', 'pg_catalog', '-N',  '"Foo"', 'postgres' ],
+	'cannot use option -n | --schema and -N | --exclude-schema at the same time');
 
 done_testing();
diff --git a/src/bin/scripts/t/101_vacuumdb_all.pl b/src/bin/scripts/t/101_vacuumdb_all.pl
index 1dcf411767..b122c995b1 100644
--- a/src/bin/scripts/t/101_vacuumdb_all.pl
+++ b/src/bin/scripts/t/101_vacuumdb_all.pl
@@ -15,5 +15,8 @@ $node->issues_sql_like(
 	[ 'vacuumdb', '-a' ],
 	qr/statement: VACUUM.*statement: VACUUM/s,
 	'vacuum all databases');
+$node->command_fails(
+	[ 'vacuumdb', '-a',  '-n', 'pg_catalog' ],
+	'cannot vacuum specific schema(s) in all databases');
 
 done_testing();
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 92f1ffe147..ce353593ce 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -46,11 +46,28 @@ typedef struct vacuumingOptions
 	bool		process_toast;
 } vacuumingOptions;
 
+enum trivalue schema_is_exclude = TRI_DEFAULT;
+
+/*
+ * The kind of object use in the command line filter.
+ *   OBJFILTER_NONE: no filter used
+ *   OBJFILTER_SCHEMA: -n | --schema or -N | --exclude-schema
+ *   OBJFILTER_TABLE: -t | --table
+ */
+enum VacObjectFilter
+{
+	OBJFILTER_NONE,
+	OBJFILTER_TABLE,
+	OBJFILTER_SCHEMA
+};
+
+enum VacObjectFilter objfilter = OBJFILTER_NONE;
+
 
 static void vacuum_one_database(ConnParams *cparams,
 								vacuumingOptions *vacopts,
 								int stage,
-								SimpleStringList *tables,
+								SimpleStringList *objects,
 								int concurrentCons,
 								const char *progname, bool echo, bool quiet);
 
@@ -72,7 +89,6 @@ static void help(const char *progname);
 #define ANALYZE_NO_STAGE	-1
 #define ANALYZE_NUM_STAGES	3
 
-
 int
 main(int argc, char *argv[])
 {
@@ -94,6 +110,8 @@ main(int argc, char *argv[])
 		{"verbose", no_argument, NULL, 'v'},
 		{"jobs", required_argument, NULL, 'j'},
 		{"parallel", required_argument, NULL, 'P'},
+		{"schema", required_argument, NULL, 'n'},
+		{"exclude-schema", required_argument, NULL, 'N'},
 		{"maintenance-db", required_argument, NULL, 2},
 		{"analyze-in-stages", no_argument, NULL, 3},
 		{"disable-page-skipping", no_argument, NULL, 4},
@@ -122,7 +140,7 @@ main(int argc, char *argv[])
 	vacuumingOptions vacopts;
 	bool		analyze_in_stages = false;
 	bool		alldb = false;
-	SimpleStringList tables = {NULL, NULL};
+	SimpleStringList objects = {NULL, NULL};
 	int			concurrentCons = 1;
 	int			tbl_count = 0;
 
@@ -140,7 +158,7 @@ main(int argc, char *argv[])
 
 	handle_help_version_opts(argc, argv, "vacuumdb", help);
 
-	while ((c = getopt_long(argc, argv, "h:p:U:wWeqd:zZFat:fvj:P:", long_options, &optindex)) != -1)
+	while ((c = getopt_long(argc, argv, "h:p:U:wWeqd:zZFat:fvj:P:n:N:", long_options, &optindex)) != -1)
 	{
 		switch (c)
 		{
@@ -182,7 +200,11 @@ main(int argc, char *argv[])
 				break;
 			case 't':
 				{
-					simple_string_list_append(&tables, optarg);
+					/* When filtering on schema name, filter by table is not allowed. */
+					if (schema_is_exclude != TRI_DEFAULT)
+						pg_fatal("cannot vacuum all tables in schema(s) and specific table(s) at the same time");
+					simple_string_list_append(&objects, optarg);
+					objfilter = OBJFILTER_TABLE;
 					tbl_count++;
 					break;
 				}
@@ -202,6 +224,32 @@ main(int argc, char *argv[])
 									  &vacopts.parallel_workers))
 					exit(1);
 				break;
+			case 'n':			/* include schema(s) */
+				{
+					/* When filtering on schema name, filter by table is not allowed. */
+					if (tbl_count)
+						pg_fatal("cannot vacuum all tables in schema(s) and specific table(s) at the same time");
+
+					if (schema_is_exclude == TRI_YES)
+						pg_fatal("cannot vacuum all tables in schema(s) and exclude specific schema(s) at the same time");
+					simple_string_list_append(&objects, optarg);
+					objfilter = OBJFILTER_SCHEMA;
+					schema_is_exclude = TRI_NO;
+					break;
+				}
+			case 'N':			/* exclude schema(s) */
+				{
+					/* When filtering on schema name, filter by table is not allowed. */
+					if (tbl_count)
+						pg_fatal("cannot vacuum all tables in schema(s) and specific table(s) at the same time");
+					if (schema_is_exclude == TRI_NO)
+						pg_fatal("cannot vacuum all tables in schema(s) and exclude specific schema(s) at the same time");
+
+					simple_string_list_append(&objects, optarg);
+					objfilter = OBJFILTER_SCHEMA;
+					schema_is_exclude = TRI_YES;
+					break;
+				}
 			case 2:
 				maintenance_db = pg_strdup(optarg);
 				break;
@@ -312,6 +360,13 @@ main(int argc, char *argv[])
 
 	setup_cancel_handler(NULL);
 
+	/*
+	 * When filtering on schema name, filter by table is not allowed.
+	 * The schema name can already be set to a fqdn table name.
+	 */
+	if (tbl_count && objfilter == OBJFILTER_SCHEMA && objects.head != NULL)
+		pg_fatal("cannot vacuum all tables in schema(s) and specific table(s) at the same time");
+
 	/* Avoid opening extra connections. */
 	if (tbl_count && (concurrentCons > tbl_count))
 		concurrentCons = tbl_count;
@@ -320,7 +375,15 @@ main(int argc, char *argv[])
 	{
 		if (dbname)
 			pg_fatal("cannot vacuum all databases and a specific one at the same time");
-		if (tables.head != NULL)
+		if (objfilter == OBJFILTER_SCHEMA && objects.head != NULL)
+		{
+			if (schema_is_exclude == TRI_YES)
+				pg_fatal("cannot exclude from vacuum specific schema(s) in all databases");
+			else if (schema_is_exclude == TRI_NO)
+				pg_fatal("cannot vacuum specific schema(s) in all databases");
+		}
+
+		if (objfilter == OBJFILTER_TABLE && objects.head != NULL)
 			pg_fatal("cannot vacuum specific table(s) in all databases");
 
 		cparams.dbname = maintenance_db;
@@ -352,7 +415,7 @@ main(int argc, char *argv[])
 			{
 				vacuum_one_database(&cparams, &vacopts,
 									stage,
-									&tables,
+									&objects,
 									concurrentCons,
 									progname, echo, quiet);
 			}
@@ -360,7 +423,7 @@ main(int argc, char *argv[])
 		else
 			vacuum_one_database(&cparams, &vacopts,
 								ANALYZE_NO_STAGE,
-								&tables,
+								&objects,
 								concurrentCons,
 								progname, echo, quiet);
 	}
@@ -385,7 +448,7 @@ static void
 vacuum_one_database(ConnParams *cparams,
 					vacuumingOptions *vacopts,
 					int stage,
-					SimpleStringList *tables,
+					SimpleStringList *objects,
 					int concurrentCons,
 					const char *progname, bool echo, bool quiet)
 {
@@ -400,7 +463,7 @@ vacuum_one_database(ConnParams *cparams,
 	int			i;
 	int			ntups;
 	bool		failed = false;
-	bool		tables_listed = false;
+	bool		objects_listed = false;
 	bool		has_where = false;
 	const char *initcmd;
 	const char *stage_commands[] = {
@@ -499,31 +562,41 @@ vacuum_one_database(ConnParams *cparams,
 	 * catalog query will fail.
 	 */
 	initPQExpBuffer(&catalog_query);
-	for (cell = tables ? tables->head : NULL; cell; cell = cell->next)
+	for (cell = objects ? objects->head : NULL; cell; cell = cell->next)
 	{
-		char	   *just_table;
-		const char *just_columns;
+		char	   *just_table = NULL;
+		const char *just_columns = NULL;
 
-		/*
-		 * Split relation and column names given by the user, this is used to
-		 * feed the CTE with values on which are performed pre-run validity
-		 * checks as well.  For now these happen only on the relation name.
-		 */
-		splitTableColumnsSpec(cell->val, PQclientEncoding(conn),
-							  &just_table, &just_columns);
-
-		if (!tables_listed)
+		if (!objects_listed)
 		{
 			appendPQExpBufferStr(&catalog_query,
-								 "WITH listed_tables (table_oid, column_list) "
+								 "WITH listed_objects (object_oid, column_list) "
 								 "AS (\n  VALUES (");
-			tables_listed = true;
+			objects_listed = true;
 		}
 		else
 			appendPQExpBufferStr(&catalog_query, ",\n  (");
 
-		appendStringLiteralConn(&catalog_query, just_table, conn);
-		appendPQExpBufferStr(&catalog_query, "::pg_catalog.regclass, ");
+
+		if (objfilter == OBJFILTER_SCHEMA)
+		{
+			appendStringLiteralConn(&catalog_query, cell->val, conn);
+			appendPQExpBufferStr(&catalog_query, "::pg_catalog.regnamespace::pg_catalog.oid, ");
+		}
+
+		if (objfilter == OBJFILTER_TABLE)
+		{
+			/*
+			 * Split relation and column names given by the user, this is used to
+			 * feed the CTE with values on which are performed pre-run validity
+			 * checks as well.  For now these happen only on the relation name.
+			 */
+			splitTableColumnsSpec(cell->val, PQclientEncoding(conn),
+								  &just_table, &just_columns);
+
+			appendStringLiteralConn(&catalog_query, just_table, conn);
+			appendPQExpBufferStr(&catalog_query, "::pg_catalog.regclass, ");
+		}
 
 		if (just_columns && just_columns[0] != '\0')
 			appendStringLiteralConn(&catalog_query, just_columns, conn);
@@ -536,13 +609,13 @@ vacuum_one_database(ConnParams *cparams,
 	}
 
 	/* Finish formatting the CTE */
-	if (tables_listed)
+	if (objects_listed)
 		appendPQExpBufferStr(&catalog_query, "\n)\n");
 
 	appendPQExpBufferStr(&catalog_query, "SELECT c.relname, ns.nspname");
 
-	if (tables_listed)
-		appendPQExpBufferStr(&catalog_query, ", listed_tables.column_list");
+	if (objects_listed)
+		appendPQExpBufferStr(&catalog_query, ", listed_objects.column_list");
 
 	appendPQExpBufferStr(&catalog_query,
 						 " FROM pg_catalog.pg_class c\n"
@@ -551,18 +624,26 @@ vacuum_one_database(ConnParams *cparams,
 						 " LEFT JOIN pg_catalog.pg_class t"
 						 " ON c.reltoastrelid OPERATOR(pg_catalog.=) t.oid\n");
 
-	/* Used to match the tables listed by the user */
-	if (tables_listed)
-		appendPQExpBufferStr(&catalog_query, " JOIN listed_tables"
-							 " ON listed_tables.table_oid OPERATOR(pg_catalog.=) c.oid\n");
+	/* Used to match the tables or schemas listed by the user */
+	if (objects_listed)
+	{
+		appendPQExpBufferStr(&catalog_query, " JOIN listed_objects"
+				 " ON listed_objects.object_oid OPERATOR(pg_catalog.=) ");
+		if (objfilter == OBJFILTER_TABLE)
+			appendPQExpBufferStr(&catalog_query, "c.oid\n");
+		if (objfilter == OBJFILTER_SCHEMA)
+			appendPQExpBufferStr(&catalog_query, "ns.oid\n");
+	}
 
 	/*
-	 * If no tables were listed, filter for the relevant relation types.  If
-	 * tables were given via --table, don't bother filtering by relation type.
-	 * Instead, let the server decide whether a given relation can be
-	 * processed in which case the user will know about it.
+	 * If no tables were listed or that a filter by schema is used, filter
+	 * for the relevant relation types.  If tables were given via --table,
+	 * don't bother filtering by relation type.  Instead, let the server
+	 * decide whether a given relation can be processed in which case the
+	 * user will know about it.  If there is a filter by schema the use of
+	 * --table is not possible so we have to filter by relation type too.
 	 */
-	if (!tables_listed)
+	if (!objects_listed || objfilter == OBJFILTER_SCHEMA)
 	{
 		appendPQExpBufferStr(&catalog_query, " WHERE c.relkind OPERATOR(pg_catalog.=) ANY (array["
 							 CppAsString2(RELKIND_RELATION) ", "
@@ -633,7 +714,7 @@ vacuum_one_database(ConnParams *cparams,
 							 fmtQualifiedId(PQgetvalue(res, i, 1),
 											PQgetvalue(res, i, 0)));
 
-		if (tables_listed && !PQgetisnull(res, i, 2))
+		if (objects_listed && !PQgetisnull(res, i, 2))
 			appendPQExpBufferStr(&buf, PQgetvalue(res, i, 2));
 
 		simple_string_list_append(&dbtables, buf.data);
@@ -977,6 +1058,8 @@ help(const char *progname)
 	printf(_("      --no-index-cleanup          don't remove index entries that point to dead tuples\n"));
 	printf(_("      --no-process-toast          skip the TOAST table associated with the table to vacuum\n"));
 	printf(_("      --no-truncate               don't truncate empty pages at the end of the table\n"));
+	printf(_("  -n, --schema=PATTERN            vacuum tables in the specified schema(s) only\n"));
+	printf(_("  -N, --exclude-schema=PATTERN    do NOT vacuum tables in the specified schema(s)\n"));
 	printf(_("  -P, --parallel=PARALLEL_WORKERS use this many background workers for vacuum, if available\n"));
 	printf(_("  -q, --quiet                     don't write any messages\n"));
 	printf(_("      --skip-locked               skip relations that cannot be immediately locked\n"));

Reply via email to