vacuum_multiple_tables_v1.patch

application/octet-stream

Filename: vacuum_multiple_tables_v1.patch
Type: application/octet-stream
Part: 0
Message: [Proposal] Allow users to specify multiple tables in VACUUM commands

Patch

Format: unified
Series: patch v1
File+
doc/src/sgml/ref/vacuum.sgml 10 8
src/backend/commands/vacuum.c 72 44
src/backend/nodes/copyfuncs.c 1 1
src/backend/nodes/equalfuncs.c 1 1
src/backend/parser/gram.y 14 14
src/backend/postmaster/autovacuum.c 2 2
src/include/commands/vacuum.h 1 1
src/include/nodes/parsenodes.h 1 1
src/test/regress/expected/vacuum.out 12 2
src/test/regress/sql/vacuum.sql 11 3
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index 421c18d..48c77a9 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -21,9 +21,9 @@ PostgreSQL documentation
 
  <refsynopsisdiv>
 <synopsis>
-VACUUM [ ( { FULL | FREEZE | VERBOSE | ANALYZE | DISABLE_PAGE_SKIPPING } [, ...] ) ] [ <replaceable class="PARAMETER">table_name</replaceable> [ (<replaceable class="PARAMETER">column_name</replaceable> [, ...] ) ] ]
-VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ <replaceable class="PARAMETER">table_name</replaceable> ]
-VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] ANALYZE [ <replaceable class="PARAMETER">table_name</replaceable> [ (<replaceable class="PARAMETER">column_name</replaceable> [, ...] ) ] ]
+VACUUM [ ( { FULL | FREEZE | VERBOSE | ANALYZE | DISABLE_PAGE_SKIPPING } [, ...] ) ] [ <replaceable class="PARAMETER">table_name</replaceable> [, ...] [ (<replaceable class="PARAMETER">column_name</replaceable> [, ...] ) ] ]
+VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ <replaceable class="PARAMETER">table_name</replaceable> [, ...] ]
+VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] ANALYZE [ <replaceable class="PARAMETER">table_name</replaceable> [, ...] [ (<replaceable class="PARAMETER">column_name</replaceable> [, ...] ) ] ]
 </synopsis>
  </refsynopsisdiv>
 
@@ -40,9 +40,10 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] ANALYZE [ <replaceable class="PARAMETER">
   </para>
 
   <para>
-   With no parameter, <command>VACUUM</command> processes every table in the
+   With no parameters, <command>VACUUM</command> processes every table in the
    current database that the current user has permission to vacuum.
-   With a parameter, <command>VACUUM</command> processes only that table.
+   With parameters, <command>VACUUM</command> processes only the tables
+   specified.
   </para>
 
   <para>
@@ -152,9 +153,9 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] ANALYZE [ <replaceable class="PARAMETER">
     <term><replaceable class="PARAMETER">table_name</replaceable></term>
     <listitem>
      <para>
-      The name (optionally schema-qualified) of a specific table to
+      The names (optionally schema-qualified) of the specific tables to
       vacuum.  If omitted, all regular tables and materialized views in the
-      current database are vacuumed.  If the specified table is a partitioned
+      current database are vacuumed.  If a specified table is a partitioned
       table, all of its leaf partitions are vacuumed.
      </para>
     </listitem>
@@ -165,7 +166,8 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] ANALYZE [ <replaceable class="PARAMETER">
     <listitem>
      <para>
       The name of a specific column to analyze. Defaults to all columns.
-      If a column list is specified, <literal>ANALYZE</> is implied.
+      If a column list is specified, only one table may be specified, and
+      <literal>ANALYZE</> is implied.
      </para>
     </listitem>
    </varlistentry>
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 9fbb0eb..ea9a203 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -67,7 +67,7 @@ static BufferAccessStrategy vac_strategy;
 
 
 /* non-export function prototypes */
-static List *get_rel_oids(Oid relid, const RangeVar *vacrel);
+static List *get_rel_oids(Oid relid, const List *vacrels);
 static void vac_truncate_clog(TransactionId frozenXID,
 				  MultiXactId minMulti,
 				  TransactionId lastSaneFrozenXid,
@@ -93,6 +93,12 @@ ExecVacuum(VacuumStmt *vacstmt, bool isTopLevel)
 	Assert((vacstmt->options & VACOPT_ANALYZE) || vacstmt->va_cols == NIL);
 	Assert(!(vacstmt->options & VACOPT_SKIPTOAST));
 
+	/* check that the tables and columns in the statement make sense */
+	if (list_length(vacstmt->va_rels) != 1 && vacstmt->va_cols != NIL)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("only one table may be specified when columns are specified")));
+
 	/*
 	 * All freeze ages are zero if the FREEZE option is given; otherwise pass
 	 * them as -1 which means to use the default values.
@@ -119,7 +125,7 @@ ExecVacuum(VacuumStmt *vacstmt, bool isTopLevel)
 	params.log_min_duration = -1;
 
 	/* Now go through the common routine */
-	vacuum(vacstmt->options, vacstmt->relation, InvalidOid, &params,
+	vacuum(vacstmt->options, vacstmt->va_rels, InvalidOid, &params,
 		   vacstmt->va_cols, NULL, isTopLevel);
 }
 
@@ -146,7 +152,7 @@ ExecVacuum(VacuumStmt *vacstmt, bool isTopLevel)
  * memory context that will not disappear at transaction commit.
  */
 void
-vacuum(int options, RangeVar *relation, Oid relid, VacuumParams *params,
+vacuum(int options, List *va_rels, Oid relid, VacuumParams *params,
 	   List *va_cols, BufferAccessStrategy bstrategy, bool isTopLevel)
 {
 	const char *stmttype;
@@ -229,7 +235,7 @@ vacuum(int options, RangeVar *relation, Oid relid, VacuumParams *params,
 	 * Build list of relations to process, unless caller gave us one. (If we
 	 * build one, we put it in vac_context for safekeeping.)
 	 */
-	relations = get_rel_oids(relid, relation);
+	relations = get_rel_oids(relid, va_rels);
 
 	/*
 	 * Decide whether we need to start/commit our own transactions.
@@ -284,6 +290,23 @@ vacuum(int options, RangeVar *relation, Oid relid, VacuumParams *params,
 	PG_TRY();
 	{
 		ListCell   *cur;
+		RangeVar   *relation = NULL;
+
+		/*
+		 * The "relation" argument in vacuum_rel(...) and analyze_rel(...) is
+		 * only needed for autovacuum workers, which call vacuum(...) with
+		 * only one relation specified.  If multiple relations are specified
+		 * (va_rels is empty or has multiple entries), then we know that we
+		 * do not need the "relation" argument, as the current caller is not
+		 * an autovacuum worker.
+		 *
+		 * This means we only need to specify a value for "relation" if we
+		 * are vacuuming exactly one relation, which simplifies things a bit.
+		 */
+		if (list_length(va_rels) == 1)
+			relation = (RangeVar *) linitial(va_rels);
+		else
+			Assert(!IsAutoVacuumWorkerProcess());
 
 		in_vacuum = true;
 		VacuumCostActive = (VacuumCostDelay > 0);
@@ -379,7 +402,7 @@ vacuum(int options, RangeVar *relation, Oid relid, VacuumParams *params,
  * per-relation transactions.
  */
 static List *
-get_rel_oids(Oid relid, const RangeVar *vacrel)
+get_rel_oids(Oid relid, const List *vacrels)
 {
 	List	   *oid_list = NIL;
 	MemoryContext oldcontext;
@@ -391,49 +414,54 @@ get_rel_oids(Oid relid, const RangeVar *vacrel)
 		oid_list = lappend_oid(oid_list, relid);
 		MemoryContextSwitchTo(oldcontext);
 	}
-	else if (vacrel)
+	else if (vacrels != NIL)
 	{
-		/* Process a specific relation */
-		Oid			relid;
-		HeapTuple	tuple;
-		Form_pg_class classForm;
-		bool		include_parts;
+		/* Process specific relation(s) */
+		ListCell *lc;
 
-		/*
-		 * Since we don't take a lock here, the relation might be gone, or the
-		 * RangeVar might no longer refer to the OID we look up here.  In the
-		 * former case, VACUUM will do nothing; in the latter case, it will
-		 * process the OID we looked up here, rather than the new one. Neither
-		 * is ideal, but there's little practical alternative, since we're
-		 * going to commit this transaction and begin a new one between now
-		 * and then.
-		 */
-		relid = RangeVarGetRelid(vacrel, NoLock, false);
+		foreach(lc, vacrels)
+		{
+			Oid			relid;
+			HeapTuple	tuple;
+			Form_pg_class classForm;
+			bool		include_parts;
 
-		/*
-		 * To check whether the relation is a partitioned table, fetch its
-		 * syscache entry.
-		 */
-		tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
-		if (!HeapTupleIsValid(tuple))
-			elog(ERROR, "cache lookup failed for relation %u", relid);
-		classForm = (Form_pg_class) GETSTRUCT(tuple);
-		include_parts = (classForm->relkind == RELKIND_PARTITIONED_TABLE);
-		ReleaseSysCache(tuple);
+			/*
+			 * Since we don't take a lock here, the relation might be gone, or the
+			 * RangeVar might no longer refer to the OID we look up here.  In the
+			 * former case, VACUUM will do nothing; in the latter case, it will
+			 * process the OID we looked up here, rather than the new one. Neither
+			 * is ideal, but there's little practical alternative, since we're
+			 * going to commit this transaction and begin a new one between now
+			 * and then.
+			 */
+			relid = RangeVarGetRelid((RangeVar *) lfirst(lc), NoLock, false);
 
-		/*
-		 * Make relation list entries for this guy and its partitions, if any.
-		 * Note that the list returned by find_all_inheritors() include the
-		 * passed-in OID at its head.  Also note that we did not request a
-		 * lock to be taken to match what would be done otherwise.
-		 */
-		oldcontext = MemoryContextSwitchTo(vac_context);
-		if (include_parts)
-			oid_list = list_concat(oid_list,
-								   find_all_inheritors(relid, NoLock, NULL));
-		else
-			oid_list = lappend_oid(oid_list, relid);
-		MemoryContextSwitchTo(oldcontext);
+			/*
+			 * To check whether the relation is a partitioned table, fetch its
+			 * syscache entry.
+			 */
+			tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
+			if (!HeapTupleIsValid(tuple))
+				elog(ERROR, "cache lookup failed for relation %u", relid);
+			classForm = (Form_pg_class) GETSTRUCT(tuple);
+			include_parts = (classForm->relkind == RELKIND_PARTITIONED_TABLE);
+			ReleaseSysCache(tuple);
+
+			/*
+			 * Make relation list entries for this guy and its partitions, if any.
+			 * Note that the list returned by find_all_inheritors() include the
+			 * passed-in OID at its head.  Also note that we did not request a
+			 * lock to be taken to match what would be done otherwise.
+			 */
+			oldcontext = MemoryContextSwitchTo(vac_context);
+			if (include_parts)
+				oid_list = list_concat(oid_list,
+									   find_all_inheritors(relid, NoLock, NULL));
+			else
+				oid_list = lappend_oid(oid_list, relid);
+			MemoryContextSwitchTo(oldcontext);
+		}
 	}
 	else
 	{
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 2d2a9d0..788421a 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3763,7 +3763,7 @@ _copyVacuumStmt(const VacuumStmt *from)
 	VacuumStmt *newnode = makeNode(VacuumStmt);
 
 	COPY_SCALAR_FIELD(options);
-	COPY_NODE_FIELD(relation);
+	COPY_NODE_FIELD(va_rels);
 	COPY_NODE_FIELD(va_cols);
 
 	return newnode;
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index b5459cd..4fe2f09 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1663,7 +1663,7 @@ static bool
 _equalVacuumStmt(const VacuumStmt *a, const VacuumStmt *b)
 {
 	COMPARE_SCALAR_FIELD(options);
-	COMPARE_NODE_FIELD(relation);
+	COMPARE_NODE_FIELD(va_rels);
 	COMPARE_NODE_FIELD(va_cols);
 
 	return true;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 65c004c..83b5bd7 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -10155,11 +10155,11 @@ VacuumStmt: VACUUM opt_full opt_freeze opt_verbose
 						n->options |= VACOPT_FREEZE;
 					if ($4)
 						n->options |= VACOPT_VERBOSE;
-					n->relation = NULL;
+					n->va_rels = NIL;
 					n->va_cols = NIL;
-					$$ = (Node *)n;
+					$$ = (Node *) n;
 				}
-			| VACUUM opt_full opt_freeze opt_verbose qualified_name
+			| VACUUM opt_full opt_freeze opt_verbose qualified_name_list
 				{
 					VacuumStmt *n = makeNode(VacuumStmt);
 					n->options = VACOPT_VACUUM;
@@ -10169,9 +10169,9 @@ VacuumStmt: VACUUM opt_full opt_freeze opt_verbose
 						n->options |= VACOPT_FREEZE;
 					if ($4)
 						n->options |= VACOPT_VERBOSE;
-					n->relation = $5;
+					n->va_rels = $5;
 					n->va_cols = NIL;
-					$$ = (Node *)n;
+					$$ = (Node *) n;
 				}
 			| VACUUM opt_full opt_freeze opt_verbose AnalyzeStmt
 				{
@@ -10183,21 +10183,21 @@ VacuumStmt: VACUUM opt_full opt_freeze opt_verbose
 						n->options |= VACOPT_FREEZE;
 					if ($4)
 						n->options |= VACOPT_VERBOSE;
-					$$ = (Node *)n;
+					$$ = (Node *) n;
 				}
 			| VACUUM '(' vacuum_option_list ')'
 				{
 					VacuumStmt *n = makeNode(VacuumStmt);
 					n->options = VACOPT_VACUUM | $3;
-					n->relation = NULL;
+					n->va_rels = NIL;
 					n->va_cols = NIL;
 					$$ = (Node *) n;
 				}
-			| VACUUM '(' vacuum_option_list ')' qualified_name opt_name_list
+			| VACUUM '(' vacuum_option_list ')' qualified_name_list opt_name_list
 				{
 					VacuumStmt *n = makeNode(VacuumStmt);
 					n->options = VACOPT_VACUUM | $3;
-					n->relation = $5;
+					n->va_rels = $5;
 					n->va_cols = $6;
 					if (n->va_cols != NIL)	/* implies analyze */
 						n->options |= VACOPT_ANALYZE;
@@ -10234,19 +10234,19 @@ AnalyzeStmt:
 					n->options = VACOPT_ANALYZE;
 					if ($2)
 						n->options |= VACOPT_VERBOSE;
-					n->relation = NULL;
+					n->va_rels = NIL;
 					n->va_cols = NIL;
-					$$ = (Node *)n;
+					$$ = (Node *) n;
 				}
-			| analyze_keyword opt_verbose qualified_name opt_name_list
+			| analyze_keyword opt_verbose qualified_name_list opt_name_list
 				{
 					VacuumStmt *n = makeNode(VacuumStmt);
 					n->options = VACOPT_ANALYZE;
 					if ($2)
 						n->options |= VACOPT_VERBOSE;
-					n->relation = $3;
+					n->va_rels = $3;
 					n->va_cols = $4;
-					$$ = (Node *)n;
+					$$ = (Node *) n;
 				}
 		;
 
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 89dd3b3..155d579 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -3125,8 +3125,8 @@ autovacuum_do_vac_analyze(autovac_table *tab, BufferAccessStrategy bstrategy)
 	/* Let pgstat know what we're doing */
 	autovac_report_activity(tab);
 
-	vacuum(tab->at_vacoptions, &rangevar, tab->at_relid, &tab->at_params, NIL,
-		   bstrategy, true);
+	vacuum(tab->at_vacoptions, list_make1(&rangevar), tab->at_relid, &tab->at_params,
+		   NIL, bstrategy, true);
 }
 
 /*
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 541c2fa..52d6e44 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -158,7 +158,7 @@ extern int	vacuum_multixact_freeze_table_age;
 
 /* in commands/vacuum.c */
 extern void ExecVacuum(VacuumStmt *vacstmt, bool isTopLevel);
-extern void vacuum(int options, RangeVar *relation, Oid relid,
+extern void vacuum(int options, List *va_rels, Oid relid,
 	   VacuumParams *params, List *va_cols,
 	   BufferAccessStrategy bstrategy, bool isTopLevel);
 extern void vac_open_indexes(Relation relation, LOCKMODE lockmode,
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 46c23c2..f8411d2 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3066,7 +3066,7 @@ typedef struct VacuumStmt
 {
 	NodeTag		type;
 	int			options;		/* OR of VacuumOption flags */
-	RangeVar   *relation;		/* single table to process, or NULL */
+	List	   *va_rels;		/* list of tables to process, or NIL for all */
 	List	   *va_cols;		/* list of column names, or NIL for all */
 } VacuumStmt;
 
diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out
index 6f68663..855ac9f 100644
--- a/src/test/regress/expected/vacuum.out
+++ b/src/test/regress/expected/vacuum.out
@@ -80,8 +80,6 @@ CONTEXT:  SQL function "do_analyze" statement 1
 SQL function "wrap_do_analyze" statement 1
 VACUUM FULL vactst;
 VACUUM (DISABLE_PAGE_SKIPPING) vaccluster;
-DROP TABLE vaccluster;
-DROP TABLE vactst;
 -- partitioned table
 CREATE TABLE vacparted (a int, b char) PARTITION BY LIST (a);
 CREATE TABLE vacparted1 PARTITION OF vacparted FOR VALUES IN (1);
@@ -90,4 +88,16 @@ UPDATE vacparted SET b = 'b';
 VACUUM (ANALYZE) vacparted;
 VACUUM (FULL) vacparted;
 VACUUM (FREEZE) vacparted;
+-- multiple tables specified
+VACUUM FREEZE vaccluster, vactst;
+VACUUM FREEZE vacparted, does_not_exist;
+ERROR:  relation "does_not_exist" does not exist
+VACUUM (FREEZE) vacparted, vaccluster, vactst;
+VACUUM (FREEZE) vaccluster, does_not_exist;
+ERROR:  relation "does_not_exist" does not exist
+VACUUM (FREEZE) vactst (i);
+VACUUM (FREEZE) vactst, vacparted (i);
+ERROR:  only one table may be specified when columns are specified
+DROP TABLE vaccluster;
+DROP TABLE vactst;
 DROP TABLE vacparted;
diff --git a/src/test/regress/sql/vacuum.sql b/src/test/regress/sql/vacuum.sql
index 7c5fb04..e1bd0b7 100644
--- a/src/test/regress/sql/vacuum.sql
+++ b/src/test/regress/sql/vacuum.sql
@@ -62,9 +62,6 @@ VACUUM FULL vactst;
 
 VACUUM (DISABLE_PAGE_SKIPPING) vaccluster;
 
-DROP TABLE vaccluster;
-DROP TABLE vactst;
-
 -- partitioned table
 CREATE TABLE vacparted (a int, b char) PARTITION BY LIST (a);
 CREATE TABLE vacparted1 PARTITION OF vacparted FOR VALUES IN (1);
@@ -73,4 +70,15 @@ UPDATE vacparted SET b = 'b';
 VACUUM (ANALYZE) vacparted;
 VACUUM (FULL) vacparted;
 VACUUM (FREEZE) vacparted;
+
+-- multiple tables specified
+VACUUM FREEZE vaccluster, vactst;
+VACUUM FREEZE vacparted, does_not_exist;
+VACUUM (FREEZE) vacparted, vaccluster, vactst;
+VACUUM (FREEZE) vaccluster, does_not_exist;
+VACUUM (FREEZE) vactst (i);
+VACUUM (FREEZE) vactst, vacparted (i);
+
+DROP TABLE vaccluster;
+DROP TABLE vactst;
 DROP TABLE vacparted;