v24-0003-Deprecate-ReindexStmt-concurrent-and-options.patch

From f8c9a93918c57e0276a4c1408c8ec35197212268 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Tue, 11 Aug 2020 01:41:09 -0500
Subject: [PATCH v24 3/7] Deprecate ReindexStmt->concurrent and options

..since parenthesized syntax is the modern syntax, remove the special field for
handling nonparenthesized, boolean-only option.

Should be squished with previous commit.
---
 src/backend/commands/cluster.c   | 11 ++++-----
 src/backend/commands/indexcmds.c | 38 +++++++++++++++++---------------
 src/backend/nodes/copyfuncs.c    |  3 ---
 src/backend/nodes/equalfuncs.c   |  3 ---
 src/backend/parser/gram.y        | 31 +++++++++++++++-----------
 src/backend/tcop/utility.c       | 21 +++++++++---------
 src/include/commands/defrem.h    |  6 ++---
 src/include/nodes/parsenodes.h   |  8 +++----
 8 files changed, 61 insertions(+), 60 deletions(-)

diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index a42dfe98f4..4a3678831d 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -105,7 +105,8 @@ static List *get_tables_to_cluster(MemoryContext cluster_context);
 void
 cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
 {
-	ListCell *lc;
+	ListCell	*lc;
+	int			options = 0;
 
 	/* Parse list of generic parameters not handled by the parser */
 	foreach(lc, stmt->params)
@@ -114,9 +115,9 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
 
 		if (strcmp(opt->defname, "verbose") == 0)
 			if (defGetBoolean(opt))
-				stmt->options |= CLUOPT_VERBOSE;
+				options |= CLUOPT_VERBOSE;
 			else
-				stmt->options &= ~CLUOPT_VERBOSE;
+				options &= ~CLUOPT_VERBOSE;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -194,7 +195,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
 		table_close(rel, NoLock);
 
 		/* Do the job. */
-		cluster_rel(tableOid, indexOid, stmt->options, isTopLevel);
+		cluster_rel(tableOid, indexOid, options, isTopLevel);
 	}
 	else
 	{
@@ -243,7 +244,7 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
 			PushActiveSnapshot(GetTransactionSnapshot());
 			/* Do the job. */
 			cluster_rel(rvtc->tableOid, rvtc->indexOid,
-						stmt->options | CLUOPT_RECHECK,
+						options | CLUOPT_RECHECK,
 						isTopLevel);
 			PopActiveSnapshot();
 			CommitTransactionCommand();
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index e8383feea7..9c9b7e72a4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2420,7 +2420,7 @@ ChooseIndexColumnNames(List *indexElems)
  *		Recreate a specific index.
  */
 void
-ReindexIndex(ReindexStmt *stmt)
+ReindexIndex(ReindexStmt *stmt, int options)
 {
 	RangeVar *indexRelation = stmt->relation;
 	struct ReindexIndexCallbackState state;
@@ -2438,10 +2438,10 @@ ReindexIndex(ReindexStmt *stmt)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.concurrent = stmt->concurrent;
+	state.concurrent = (options & REINDEXOPT_CONCURRENT);
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  stmt->concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
+									  state.concurrent ? ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
 									  &state);
@@ -2461,11 +2461,11 @@ ReindexIndex(ReindexStmt *stmt)
 	persistence = irel->rd_rel->relpersistence;
 	index_close(irel, NoLock);
 
-	if (stmt->concurrent && persistence != RELPERSISTENCE_TEMP)
-		ReindexRelationConcurrently(indOid, stmt->options);
+	if (state.concurrent && persistence != RELPERSISTENCE_TEMP)
+		ReindexRelationConcurrently(indOid, options);
 	else
 		reindex_index(indOid, false, persistence,
-					  stmt->options | REINDEXOPT_REPORT_PROGRESS);
+					  options | REINDEXOPT_REPORT_PROGRESS);
 }
 
 /*
@@ -2543,11 +2543,12 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
  *		Recreate all indexes of a table (and of its toast table, if any)
  */
 Oid
-ReindexTable(ReindexStmt *stmt)
+ReindexTable(ReindexStmt *stmt, int options)
 {
 	RangeVar *relation = stmt->relation;
 	Oid			heapOid;
 	bool		result;
+	bool		concurrent = (options & REINDEXOPT_CONCURRENT);
 
 	/*
 	 * The lock level used here should match reindex_relation().
@@ -2558,13 +2559,13 @@ ReindexTable(ReindexStmt *stmt)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   stmt->concurrent ? ShareUpdateExclusiveLock : ShareLock,
+									   concurrent ? ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
-	if (stmt->concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
+	if (concurrent && get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
-		result = ReindexRelationConcurrently(heapOid, stmt->options);
+		result = ReindexRelationConcurrently(heapOid, options);
 
 		if (!result)
 			ereport(NOTICE,
@@ -2576,7 +2577,7 @@ ReindexTable(ReindexStmt *stmt)
 		result = reindex_relation(heapOid,
 								  REINDEX_REL_PROCESS_TOAST |
 								  REINDEX_REL_CHECK_CONSTRAINTS,
-								  stmt->options | REINDEXOPT_REPORT_PROGRESS);
+								  options | REINDEXOPT_REPORT_PROGRESS);
 		if (!result)
 			ereport(NOTICE,
 					(errmsg("table \"%s\" has no indexes to reindex",
@@ -2595,7 +2596,7 @@ ReindexTable(ReindexStmt *stmt)
  * That means this must not be called within a user transaction block!
  */
 void
-ReindexMultipleTables(ReindexStmt *stmt)
+ReindexMultipleTables(ReindexStmt *stmt, int options)
 {
 	const char *objectName = stmt->name;
 	ReindexObjectType objectKind = stmt->kind;
@@ -2610,13 +2611,14 @@ ReindexMultipleTables(ReindexStmt *stmt)
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
+	bool		concurrent = (options & REINDEXOPT_CONCURRENT);
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
-	if (objectKind == REINDEX_OBJECT_SYSTEM && stmt->concurrent)
+	if (objectKind == REINDEX_OBJECT_SYSTEM && concurrent)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2727,7 +2729,7 @@ ReindexMultipleTables(ReindexStmt *stmt)
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if (stmt->concurrent &&
+		if (concurrent &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2777,10 +2779,10 @@ ReindexMultipleTables(ReindexStmt *stmt)
 			continue;
 		}
 
-		if (stmt->concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
+		if (concurrent && get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 		{
 			(void) ReindexRelationConcurrently(relid,
-											   stmt->options |
+											   options |
 											   REINDEXOPT_MISSING_OK);
 			/* ReindexRelationConcurrently() does the verbose output */
 		}
@@ -2791,11 +2793,11 @@ ReindexMultipleTables(ReindexStmt *stmt)
 			result = reindex_relation(relid,
 									  REINDEX_REL_PROCESS_TOAST |
 									  REINDEX_REL_CHECK_CONSTRAINTS,
-									  stmt->options |
+									  options |
 									  REINDEXOPT_REPORT_PROGRESS |
 									  REINDEXOPT_MISSING_OK);
 
-			if (result && (stmt->options & REINDEXOPT_VERBOSE))
+			if (result && (options & REINDEXOPT_VERBOSE))
 				ereport(INFO,
 						(errmsg("table \"%s.%s\" was reindexed",
 								get_namespace_name(get_rel_namespace(relid)),
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 71548acc0c..2a73f1cb6d 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3357,7 +3357,6 @@ _copyClusterStmt(const ClusterStmt *from)
 
 	COPY_NODE_FIELD(relation);
 	COPY_STRING_FIELD(indexname);
-	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
 
 	return newnode;
@@ -4450,9 +4449,7 @@ _copyReindexStmt(const ReindexStmt *from)
 	COPY_SCALAR_FIELD(kind);
 	COPY_NODE_FIELD(relation);
 	COPY_STRING_FIELD(name);
-	COPY_SCALAR_FIELD(options);
 	COPY_NODE_FIELD(params);
-	COPY_SCALAR_FIELD(concurrent);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index de48a42cdd..3c6c40a50d 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1216,7 +1216,6 @@ _equalClusterStmt(const ClusterStmt *a, const ClusterStmt *b)
 {
 	COMPARE_NODE_FIELD(relation);
 	COMPARE_STRING_FIELD(indexname);
-	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
 
 	return true;
@@ -2135,9 +2134,7 @@ _equalReindexStmt(const ReindexStmt *a, const ReindexStmt *b)
 	COMPARE_SCALAR_FIELD(kind);
 	COMPARE_NODE_FIELD(relation);
 	COMPARE_STRING_FIELD(name);
-	COMPARE_SCALAR_FIELD(options);
 	COMPARE_NODE_FIELD(params);
-	COMPARE_SCALAR_FIELD(concurrent);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index c5be94af45..525dfb8132 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8176,40 +8176,48 @@ ReindexStmt:
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->relation = $4;
 					n->name = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $2;
-					n->concurrent = $3;
 					n->name = $4;
 					n->relation = NULL;
 					n->params = NIL;
+					if ($3)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @3));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_type opt_concurrently qualified_name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->relation = $7;
 					n->name = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 			| REINDEX '(' vac_analyze_option_list ')' reindex_target_multitable opt_concurrently name
 				{
 					ReindexStmt *n = makeNode(ReindexStmt);
 					n->kind = $5;
-					n->concurrent = $6;
 					n->name = $7;
 					n->relation = NULL;
 					n->params = $3;
+					if ($6)
+						n->params = lappend(n->params,
+								makeDefElem("concurrently", (Node *)makeString("concurrently"), @6));
 					$$ = (Node *)n;
 				}
 		;
@@ -10384,10 +10392,9 @@ ClusterStmt:
 					ClusterStmt *n = makeNode(ClusterStmt);
 					n->relation = $3;
 					n->indexname = $4;
-					n->options = 0;
-					if ($2)
-						n->options |= CLUOPT_VERBOSE;
 					n->params = NIL;
+					if ($2)
+						n->params = lappend(n->params, makeDefElem("verbose", NULL, @2));
 					$$ = (Node*)n;
 				}
 
@@ -10404,10 +10411,9 @@ ClusterStmt:
 					ClusterStmt *n = makeNode(ClusterStmt);
 					n->relation = NULL;
 					n->indexname = NULL;
-					n->options = 0;
-					if ($2)
-						n->options |= CLUOPT_VERBOSE;
 					n->params = NIL;
+					if ($2)
+						n->params = lappend(n->params, makeDefElem("verbose", NULL, @2));
 					$$ = (Node*)n;
 				}
 			/* kept for pre-8.3 compatibility */
@@ -10416,10 +10422,9 @@ ClusterStmt:
 					ClusterStmt *n = makeNode(ClusterStmt);
 					n->relation = $5;
 					n->indexname = $3;
-					n->options = 0;
-					if ($2)
-						n->options |= CLUOPT_VERBOSE;
 					n->params = NIL;
+					if ($2)
+						n->params = lappend(n->params, makeDefElem("verbose", NULL, @2));
 					$$ = (Node*)n;
 				}
 		;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 0e074d7745..0b21a6cc53 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -528,7 +528,7 @@ ProcessUtility(PlannedStmt *pstmt,
 
 /* Parse params not parsed by the grammar */
 static
-void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
+void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt, int *options)
 {
 	ListCell *lc;
 	foreach(lc, stmt->params)
@@ -538,12 +538,12 @@ void parse_reindex_params(ParseState *pstate, ReindexStmt *stmt)
 		if (strcmp(opt->defname, "verbose") == 0)
 		{
 			if (defGetBoolean(opt))
-				stmt->options |= REINDEXOPT_VERBOSE;
+				*options |= REINDEXOPT_VERBOSE;
 			else
-				stmt->options &= ~REINDEXOPT_VERBOSE;
+				*options &= ~REINDEXOPT_VERBOSE;
 		}
-		else if (strcmp(opt->defname, "concurrently") == 0)
-			stmt->concurrent = true;
+		else if (strcmp(opt->defname, "concurrently") == 0) // GetBoolean ?
+			*options |= REINDEXOPT_CONCURRENT;
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -945,19 +945,20 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 		case T_ReindexStmt:
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
+				int options = 0;
 
-				if (stmt->concurrent)
+				parse_reindex_params(pstate, stmt, &options);
+				if (options & REINDEXOPT_CONCURRENT)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
-				parse_reindex_params(pstate, stmt);
 				switch (stmt->kind)
 				{
 					case REINDEX_OBJECT_INDEX:
-						ReindexIndex(stmt);
+						ReindexIndex(stmt, options);
 						break;
 					case REINDEX_OBJECT_TABLE:
-						ReindexTable(stmt);
+						ReindexTable(stmt, options);
 						break;
 					case REINDEX_OBJECT_SCHEMA:
 					case REINDEX_OBJECT_SYSTEM:
@@ -973,7 +974,7 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 												  (stmt->kind == REINDEX_OBJECT_SCHEMA) ? "REINDEX SCHEMA" :
 												  (stmt->kind == REINDEX_OBJECT_SYSTEM) ? "REINDEX SYSTEM" :
 												  "REINDEX DATABASE");
-						ReindexMultipleTables(stmt);
+						ReindexMultipleTables(stmt, options);
 						break;
 					default:
 						elog(ERROR, "unrecognized object type: %d",
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index a97c00a02e..fedbbc17e0 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -34,9 +34,9 @@ extern ObjectAddress DefineIndex(Oid relationId,
 								 bool check_not_in_use,
 								 bool skip_build,
 								 bool quiet);
-extern void ReindexIndex(ReindexStmt *stmt);
-extern Oid	ReindexTable(ReindexStmt *stmt);
-extern void ReindexMultipleTables(ReindexStmt *stmt);
+extern void ReindexIndex(ReindexStmt *stmt, int options);
+extern Oid	ReindexTable(ReindexStmt *stmt, int options);
+extern void ReindexMultipleTables(ReindexStmt *stmt, int options);
 extern char *makeObjectName(const char *name1, const char *name2,
 							const char *label);
 extern char *ChooseRelationName(const char *name1, const char *name2,
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 4e3188ff87..a524402b91 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3210,8 +3210,7 @@ typedef struct ClusterStmt
 	NodeTag		type;
 	RangeVar   *relation;		/* relation being indexed, or NULL if all */
 	char	   *indexname;		/* original index defined */
-	int			options;		/* OR of ClusterOption flags */
-	List		*params;		/* Params not further parsed by the grammar */
+	List		*params;		/* list of DefElem nodes */
 } ClusterStmt;
 
 /* ----------------------
@@ -3354,6 +3353,7 @@ typedef struct ConstraintsSetStmt
 #define REINDEXOPT_VERBOSE (1 << 0) /* print progress info */
 #define REINDEXOPT_REPORT_PROGRESS (1 << 1) /* report pgstat progress */
 #define REINDEXOPT_MISSING_OK (1 << 2)	/* skip missing relations */
+#define REINDEXOPT_CONCURRENT (1 << 3) /* reindex CONCURRENTLY */
 
 typedef enum ReindexObjectType
 {
@@ -3371,9 +3371,7 @@ typedef struct ReindexStmt
 								 * etc. */
 	RangeVar   *relation;		/* Table or index to reindex */
 	const char *name;			/* name of database to reindex */
-	int			options;		/* Reindex options flags */
-	List		*params;		/* Params not further parsed by the grammer */
-	bool		concurrent;		/* reindex concurrently? */
+	List		*params;		/* list of DefElem nodes */
 } ReindexStmt;
 
 /* ----------------------
-- 
2.19.1