From 8e9caa318c877871f2bab2f096d2406121166f3b Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Thu, 26 Mar 2020 22:08:40 -0500
Subject: [PATCH v16 6/7] fix!Parenthesized syntax: VACUUM/CLUSTER (TABLESPACE)

---
 doc/src/sgml/ref/cluster.sgml             | 19 ++++++++-
 doc/src/sgml/ref/vacuum.sgml              | 14 ++++++-
 src/backend/commands/cluster.c            | 19 +++++++++
 src/backend/commands/vacuum.c             | 51 ++++++++++++-----------
 src/backend/nodes/copyfuncs.c             |  1 -
 src/backend/nodes/equalfuncs.c            |  1 -
 src/backend/parser/gram.y                 | 36 +---------------
 src/include/nodes/parsenodes.h            |  2 -
 src/test/regress/input/tablespace.source  | 14 +++----
 src/test/regress/output/tablespace.source | 14 +++----
 10 files changed, 90 insertions(+), 81 deletions(-)

diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml
index a61a77a014..0e81e6189b 100644
--- a/doc/src/sgml/ref/cluster.sgml
+++ b/doc/src/sgml/ref/cluster.sgml
@@ -21,8 +21,14 @@ PostgreSQL documentation
 
  <refsynopsisdiv>
 <synopsis>
-CLUSTER [VERBOSE] <replaceable class="parameter">table_name</replaceable> [ USING <replaceable class="parameter">index_name</replaceable> ] [ TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> ]
+CLUSTER [VERBOSE] [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] <replaceable class="parameter">table_name</replaceable> [ USING <replaceable class="parameter">index_name</replaceable> ]
 CLUSTER [VERBOSE]
+
+<phrase>where <replaceable class="parameter">option</replaceable> can be one of:</phrase>
+
+    VERBOSE
+    TABLESPACE <replaceable class="parameter">new_tablespace</replaceable>
+
 </synopsis>
  </refsynopsisdiv>
 
@@ -90,6 +96,15 @@ CLUSTER [VERBOSE]
     </listitem>
    </varlistentry>
 
+   <varlistentry>
+    <term><literal>TABLESPACE</literal></term>
+    <listitem>
+     <para>
+      Specifies that the relation will be rebuilt on a new tablespace.
+     </para>
+    </listitem>
+   </varlistentry>
+
    <varlistentry>
     <term><replaceable class="parameter">table_name</replaceable></term>
     <listitem>
@@ -112,7 +127,7 @@ CLUSTER [VERBOSE]
     <term><replaceable class="parameter">new_tablespace</replaceable></term>
     <listitem>
      <para>
-      The tablespace where the clustered relation will be rebuilt.
+      The tablespace where the relation will be rebuilt.
      </para>
     </listitem>
    </varlistentry>
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index 681486bb38..b44b10c783 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -22,8 +22,8 @@ PostgreSQL documentation
  <refsynopsisdiv>
 <synopsis>
 VACUUM [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] [ <replaceable class="parameter">table_and_columns</replaceable> [, ...] ]
-VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> ] [ <replaceable class="parameter">table_and_columns</replaceable> [, ...] ]
-VACUUM ( FULL [, ...] ) [ TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> ] [ <replaceable class="parameter">table_and_columns</replaceable> [, ...] ]
+VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="parameter">table_and_columns</replaceable> [, ...] ]
+VACUUM ( FULL [, ...] ) [ <replaceable class="parameter">table_and_columns</replaceable> [, ...] ]
 
 <phrase>where <replaceable class="parameter">option</replaceable> can be one of:</phrase>
 
@@ -36,6 +36,7 @@ VACUUM ( FULL [, ...] ) [ TABLESPACE <replaceable class="parameter">new_tablespa
     INDEX_CLEANUP [ <replaceable class="parameter">boolean</replaceable> ]
     TRUNCATE [ <replaceable class="parameter">boolean</replaceable> ]
     PARALLEL <replaceable class="parameter">integer</replaceable>
+    TABLESPACE <replaceable class="parameter">new_tablespace</replaceable>
 
 <phrase>and <replaceable class="parameter">table_and_columns</replaceable> is:</phrase>
 
@@ -256,6 +257,15 @@ VACUUM ( FULL [, ...] ) [ TABLESPACE <replaceable class="parameter">new_tablespa
     </listitem>
    </varlistentry>
 
+   <varlistentry>
+    <term><literal>TABLESPACE</literal></term>
+    <listitem>
+     <para>
+      Specifies that the relation will be rebuilt on a new tablespace.
+     </para>
+    </listitem>
+   </varlistentry>
+
    <varlistentry>
     <term><replaceable class="parameter">boolean</replaceable></term>
     <listitem>
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index ef8b1c9a82..cef4beb2c8 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -105,6 +105,9 @@ void
 cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
 {
 	ListCell *lc;
+	/* Name and Oid of tablespace to use for clustered relation. */
+	char	*tablespaceName = NULL;
+	Oid tablespaceOid = InvalidOid;
 
 	/* Parse list of generic parameter not handled by the parser */
 	foreach(lc, stmt->params)
@@ -114,6 +117,9 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
 		if (strcmp(opt->defname, "verbose") == 0)
 			stmt->options |= CLUOPT_VERBOSE;
 			// XXX: handle boolean opt: VERBOSE off
+		else if (strcmp(opt->defname, "tablespace") == 0)
+			tablespaceName = defGetString(opt);
+			// XXX: if (tablespaceOid == GLOBALTABLESPACE_OID)
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -122,6 +128,19 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
 					 parser_errposition(pstate, opt->location)));
 	}
 
+	/* Select tablespace Oid to use. */
+	if (tablespaceName)
+	{
+		tablespaceOid = get_tablespace_oid(tablespaceName, false);
+
+		/* Can't move a non-shared relation into pg_global */
+		if (tablespaceOid == GLOBALTABLESPACE_OID)
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("cannot move non-shared relation to tablespace \"%s\"",
+							tablespaceName)));
+	}
+
 	if (stmt->relation != NULL)
 	{
 		/* This is the single-relation case. */
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 95bec8d6ba..005fb97a7b 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -109,8 +109,10 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 	bool		disable_page_skipping = false;
 	bool		parallel_option = false;
 	ListCell   *lc;
-	Oid 		tablespaceOid = InvalidOid; /* Oid of tablespace to use for relations
-											 * after VACUUM FULL. */
+
+	/* Name and Oid of tablespace to use for relations after VACUUM FULL. */
+	char		*tablespacename = NULL;
+	Oid 		tablespaceOid = InvalidOid;
 
 	/* Set default value */
 	params.index_cleanup = VACOPT_TERNARY_DEFAULT;
@@ -148,6 +150,8 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 			params.index_cleanup = get_vacopt_ternary_value(opt);
 		else if (strcmp(opt->defname, "truncate") == 0)
 			params.truncate = get_vacopt_ternary_value(opt);
+		else if (strcmp(opt->defname, "tablespace") == 0)
+			tablespacename = defGetString(opt);
 		else if (strcmp(opt->defname, "parallel") == 0)
 		{
 			parallel_option = true;
@@ -209,6 +213,27 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot specify both FULL and PARALLEL options")));
 
+	/* Get tablespace Oid to use. */
+	if (tablespacename)
+	{
+		if ((params.options & VACOPT_FULL) == 0)
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					errmsg("incompatible TABLESPACE option"),
+					errdetail("You can only use TABLESPACE with VACUUM FULL.")));
+
+		tablespaceOid = get_tablespace_oid(tablespacename, false);
+
+		/* Can't move a non-shared relation into pg_global */
+		if (tablespaceOid == GLOBALTABLESPACE_OID)
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					errmsg("cannot move non-shared relation to tablespace \"%s\"",
+							tablespacename)));
+
+	}
+	params.tablespace_oid = tablespaceOid;
+
 	/*
 	 * Make sure VACOPT_ANALYZE is specified if any column lists are present.
 	 */
@@ -246,28 +271,6 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 		params.multixact_freeze_table_age = -1;
 	}
 
-	/* Get tablespace Oid to use. */
-	if (vacstmt->tablespacename)
-	{
-		if (params.options & VACOPT_FULL)
-		{
-			tablespaceOid = get_tablespace_oid(vacstmt->tablespacename, false);
-
-			/* Can't move a non-shared relation into pg_global */
-			if (tablespaceOid == GLOBALTABLESPACE_OID)
-				ereport(ERROR,
-						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-						errmsg("cannot move non-shared relation to tablespace \"%s\"",
-								vacstmt->tablespacename)));
-		}
-		else
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					errmsg("incompatible TABLESPACE option"),
-					errdetail("You can only use TABLESPACE with VACUUM FULL.")));
-	}
-	params.tablespace_oid = tablespaceOid;
-
 	/* user-invoked vacuum is never "for wraparound" */
 	params.is_wraparound = false;
 
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index e04ee775c2..a2aa687771 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3903,7 +3903,6 @@ _copyVacuumStmt(const VacuumStmt *from)
 	COPY_NODE_FIELD(options);
 	COPY_NODE_FIELD(rels);
 	COPY_SCALAR_FIELD(is_vacuumcmd);
-	COPY_STRING_FIELD(tablespacename);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 9ffad8be90..3967d0ce08 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1703,7 +1703,6 @@ _equalVacuumStmt(const VacuumStmt *a, const VacuumStmt *b)
 	COMPARE_NODE_FIELD(options);
 	COMPARE_NODE_FIELD(rels);
 	COMPARE_SCALAR_FIELD(is_vacuumcmd);
-	COMPARE_SCALAR_FIELD(tablespacename);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index a91617c4e2..c152d78846 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -10626,7 +10626,7 @@ CreateConversionStmt:
  *****************************************************************************/
 
 ClusterStmt:
-			CLUSTER opt_verbose qualified_name cluster_index_specification OptTableSpace
+			CLUSTER opt_verbose qualified_name cluster_index_specification
 				{
 					ClusterStmt *n = makeNode(ClusterStmt);
 					n->relation = $3;
@@ -10707,28 +10707,6 @@ VacuumStmt: VACUUM opt_full opt_freeze opt_verbose opt_analyze opt_vacuum_relati
 											 makeDefElem("analyze", NULL, @5));
 					n->rels = $6;
 					n->is_vacuumcmd = true;
-					n->tablespacename = NULL;
-					$$ = (Node *)n;
-				}
-			| VACUUM opt_full opt_freeze opt_verbose opt_analyze TABLESPACE name opt_vacuum_relation_list
-				{
-					VacuumStmt *n = makeNode(VacuumStmt);
-					n->options = NIL;
-					if ($2)
-						n->options = lappend(n->options,
-								makeDefElem("full", NULL, @2));
-					if ($3)
-						n->options = lappend(n->options,
-								makeDefElem("freeze", NULL, @3));
-					if ($4)
-						n->options = lappend(n->options,
-								makeDefElem("verbose", NULL, @4));
-					if ($5)
-						n->options = lappend(n->options,
-								makeDefElem("analyze", NULL, @5));
-					n->tablespacename = $7;
-					n->rels = $8;
-					n->is_vacuumcmd = true;
 					$$ = (Node *)n;
 				}
 			| VACUUM '(' vac_analyze_option_list ')' opt_vacuum_relation_list
@@ -10737,16 +10715,6 @@ VacuumStmt: VACUUM opt_full opt_freeze opt_verbose opt_analyze opt_vacuum_relati
 					n->options = $3;
 					n->rels = $5;
 					n->is_vacuumcmd = true;
-					n->tablespacename = NULL;
-					$$ = (Node *) n;
-				}
-			| VACUUM '(' vac_analyze_option_list ')' TABLESPACE name opt_vacuum_relation_list
-				{
-					VacuumStmt *n = makeNode(VacuumStmt);
-					n->options = $3;
-					n->tablespacename = $6;
-					n->rels = $7;
-					n->is_vacuumcmd = true;
 					$$ = (Node *) n;
 				}
 		;
@@ -10760,7 +10728,6 @@ AnalyzeStmt: analyze_keyword opt_verbose opt_vacuum_relation_list
 											 makeDefElem("verbose", NULL, @2));
 					n->rels = $3;
 					n->is_vacuumcmd = false;
-					n->tablespacename = NULL;
 					$$ = (Node *)n;
 				}
 			| analyze_keyword '(' vac_analyze_option_list ')' opt_vacuum_relation_list
@@ -10769,7 +10736,6 @@ AnalyzeStmt: analyze_keyword opt_verbose opt_vacuum_relation_list
 					n->options = $3;
 					n->rels = $5;
 					n->is_vacuumcmd = false;
-					n->tablespacename = NULL;
 					$$ = (Node *) n;
 				}
 		;
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 2236aaa2dc..21b1f51950 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3203,7 +3203,6 @@ typedef struct ClusterStmt
 	NodeTag		type;
 	RangeVar   *relation;		/* relation being indexed, or NULL if all */
 	char	   *indexname;		/* original index defined */
-	char	   *tablespacename; /* tablespace name to use for clustered relation */
 	int			options;		/* OR of ClusterOption flags */
 	List		*params;		/* Params not further parsed by the grammar */
 } ClusterStmt;
@@ -3221,7 +3220,6 @@ typedef struct VacuumStmt
 	List	   *options;		/* list of DefElem nodes */
 	List	   *rels;			/* list of VacuumRelation, or NIL for all */
 	bool		is_vacuumcmd;	/* true for VACUUM, false for ANALYZE */
-	char	   *tablespacename; /* tablespace name to use for vacuumed relation */
 } VacuumStmt;
 
 /*
diff --git a/src/test/regress/input/tablespace.source b/src/test/regress/input/tablespace.source
index 03c5fb9e9e..6942775bfb 100644
--- a/src/test/regress/input/tablespace.source
+++ b/src/test/regress/input/tablespace.source
@@ -47,9 +47,9 @@ WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspa
 ORDER BY relname;
 
 -- check CLUSTER with TABLESPACE change
-CLUSTER regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx TABLESPACE regress_tblspace; -- ok
-CLUSTER pg_authid USING pg_authid_rolname_index TABLESPACE regress_tblspace; -- fail
-CLUSTER regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx TABLESPACE pg_global; -- fail
+CLUSTER (TABLESPACE regress_tblspace) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- ok
+CLUSTER (TABLESPACE regress_tblspace) pg_authid USING pg_authid_rolname_index; -- fail
+CLUSTER (TABLESPACE pg_global) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- fail
 
 -- check that all relations moved to new tablespace
 SELECT relname FROM pg_class
@@ -57,10 +57,10 @@ WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspa
 ORDER BY relname;
 
 -- check VACUUM with TABLESPACE change
-VACUUM (FULL, ANALYSE, FREEZE) TABLESPACE pg_default regress_tblspace_test_tbl; -- ok
-VACUUM (FULL) TABLESPACE pg_default pg_authid; -- skip with warning
-VACUUM (ANALYSE) TABLESPACE pg_default; -- fail
-VACUUM (FULL) TABLESPACE pg_global regress_tblspace_test_tbl; -- fail
+VACUUM (FULL, ANALYSE, FREEZE, TABLESPACE pg_default) regress_tblspace_test_tbl; -- ok
+VACUUM (FULL, TABLESPACE pg_default) pg_authid; -- skip with warning
+VACUUM (ANALYSE, TABLESPACE pg_default); -- fail
+VACUUM (FULL, TABLESPACE pg_global) regress_tblspace_test_tbl; -- fail
 
 -- check that all tables moved back to pg_default
 SELECT relname FROM pg_class
diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source
index c27fa70143..2825984394 100644
--- a/src/test/regress/output/tablespace.source
+++ b/src/test/regress/output/tablespace.source
@@ -60,10 +60,10 @@ ORDER BY relname;
 (1 row)
 
 -- check CLUSTER with TABLESPACE change
-CLUSTER regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx TABLESPACE regress_tblspace; -- ok
-CLUSTER pg_authid USING pg_authid_rolname_index TABLESPACE regress_tblspace; -- fail
+CLUSTER (TABLESPACE regress_tblspace) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- ok
+CLUSTER (TABLESPACE regress_tblspace) pg_authid USING pg_authid_rolname_index; -- fail
 ERROR:  cannot cluster a shared catalog
-CLUSTER regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx TABLESPACE pg_global; -- fail
+CLUSTER (TABLESPACE pg_global) regress_tblspace_test_tbl USING regress_tblspace_test_tbl_idx; -- fail
 ERROR:  cannot move non-shared relation to tablespace "pg_global"
 -- check that all relations moved to new tablespace
 SELECT relname FROM pg_class
@@ -76,14 +76,14 @@ ORDER BY relname;
 (2 rows)
 
 -- check VACUUM with TABLESPACE change
-VACUUM (FULL, ANALYSE, FREEZE) TABLESPACE pg_default regress_tblspace_test_tbl; -- ok
-VACUUM (FULL) TABLESPACE pg_default pg_authid; -- skip with warning
+VACUUM (FULL, ANALYSE, FREEZE, TABLESPACE pg_default) regress_tblspace_test_tbl; -- ok
+VACUUM (FULL, TABLESPACE pg_default) pg_authid; -- skip with warning
 WARNING:  skipping tablespace change of "pg_authid"
 DETAIL:  Cannot move system relation, only VACUUM is performed.
-VACUUM (ANALYSE) TABLESPACE pg_default; -- fail
+VACUUM (ANALYSE, TABLESPACE pg_default); -- fail
 ERROR:  incompatible TABLESPACE option
 DETAIL:  You can only use TABLESPACE with VACUUM FULL.
-VACUUM (FULL) TABLESPACE pg_global regress_tblspace_test_tbl; -- fail
+VACUUM (FULL, TABLESPACE pg_global) regress_tblspace_test_tbl; -- fail
 ERROR:  cannot move non-shared relation to tablespace "pg_global"
 -- check that all tables moved back to pg_default
 SELECT relname FROM pg_class
-- 
2.17.0

