Subject: [PATCH v32 1/5] ExecReindex and ReindexParams

From: Justin Pryzby <[email-masked]>
Date: Wed, 2 Dec 2020 20:54:47 -0600

TODO: typedef
---
 src/backend/commands/indexcmds.c | 151 ++++++++++++++++++++-----------
 src/backend/tcop/utility.c       |  40 +-------
 src/include/commands/defrem.h    |   7 +-
 3 files changed, 101 insertions(+), 97 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 14d24b3cc4..f0456dcbef 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -67,6 +67,10 @@
 #include "utils/syscache.h"

+typedef struct ReindexParams {
+	int options;	/* bitmask of lowlevel REINDEXOPT_* */
+} ReindexParams;
+
 /* non-export function prototypes */
 static bool CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts);
 static void CheckPredicate(Expr *predicate);
@@ -86,12 +90,17 @@ static char *ChooseIndexName(const char *tabname, Oid namespaceId,
 							 bool primary, bool isconstraint);
 static char *ChooseIndexNameAddition(List *colnames);
 static List *ChooseIndexColumnNames(List *indexElems);
+
+static void ReindexIndex(RangeVar *indexRelation, ReindexParams *params, bool isTopLevel);
+static Oid ReindexTable(RangeVar *relation, ReindexParams *params, bool isTopLevel);
+static void ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, ReindexParams *params);
+
 static void RangeVarCallbackForReindexIndex(const RangeVar *relation,
 											Oid relId, Oid oldRelId, void *arg);
 static void reindex_error_callback(void *args);
-static void ReindexPartitions(Oid relid, int options, bool isTopLevel);
-static void ReindexMultipleInternal(List *relids, int options);
-static bool ReindexRelationConcurrently(Oid relationOid, int options);
+static void ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel);
+static void ReindexMultipleInternal(List *relids, ReindexParams *params);
+static bool ReindexRelationConcurrently(Oid relationOid, ReindexParams *params);
 static void update_relispartition(Oid relationId, bool newval);
 static inline void set_indexsafe_procflags(void);
 
@@ -100,7 +109,7 @@ static inline void set_indexsafe_procflags(void);
  */
 struct ReindexIndexCallbackState
 {
-	int			options;		/* options from statement */
+	ReindexParams		*params;
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2452,16 +2461,19 @@ ChooseIndexColumnNames(List *indexElems)
 }
 
 /*
- * ReindexParseOptions
- *		Parse list of REINDEX options, returning a bitmask of ReindexOption.
+ * Reindex accordinging to stmt.
+ * This calls the intermediate routines: ReindexIndex, ReindexTable, ReindexMultipleTables,
+ * which ultimately call reindex_index, reindex_relation, ReindexRelationConcurrently.
+ * Note that partitioned relations are handled by ReindexPartitions, except that
+ * ReindexRelationConcurrently handles concurrently reindexing a table.
  */
-int
-ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt)
+void
+ExecReindex(ParseState *pstate, ReindexStmt *stmt, bool isTopLevel)
 {
-	ListCell   *lc;
-	int			options = 0;
-	bool		concurrently = false;
-	bool		verbose = false;
+	ReindexParams		params = {0};
+	bool		verbose = false,
+				concurrently = false;
+	ListCell   	*lc;
 
 	/* Parse option list */
 	foreach(lc, stmt->params)
@@ -2480,19 +2492,53 @@ ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt)
 					 parser_errposition(pstate, opt->location)));
 	}
 
-	options =
-		(verbose ? REINDEXOPT_VERBOSE : 0) |
-		(concurrently ? REINDEXOPT_CONCURRENTLY : 0);
+	if (verbose)
+		params.options |= REINDEXOPT_VERBOSE;
+
+	if (concurrently)
+	{
+		params.options |= REINDEXOPT_CONCURRENTLY;
+		PreventInTransactionBlock(isTopLevel,
+								  "REINDEX CONCURRENTLY");
+	}
+
+	switch (stmt->kind)
+	{
+		case REINDEX_OBJECT_INDEX:
+			ReindexIndex(stmt->relation, &params, isTopLevel);
+			break;
+		case REINDEX_OBJECT_TABLE:
+			ReindexTable(stmt->relation, &params, isTopLevel);
+			break;
+		case REINDEX_OBJECT_SCHEMA:
+		case REINDEX_OBJECT_SYSTEM:
+		case REINDEX_OBJECT_DATABASE:
 
-	return options;
+			/*
+			 * This cannot run inside a user transaction block; if
+			 * we were inside a transaction, then its commit- and
+			 * start-transaction-command calls would not have the
+			 * intended effect!
+			 */
+			PreventInTransactionBlock(isTopLevel,
+									  (stmt->kind == REINDEX_OBJECT_SCHEMA) ? "REINDEX SCHEMA" :
+									  (stmt->kind == REINDEX_OBJECT_SYSTEM) ? "REINDEX SYSTEM" :
+									  "REINDEX DATABASE");
+			ReindexMultipleTables(stmt->name, stmt->kind, &params);
+			break;
+		default:
+			elog(ERROR, "unrecognized object type: %d",
+				 (int) stmt->kind);
+			break;
+	}
 }
 
 /*
  * ReindexIndex
  *		Recreate a specific index.
  */
-void
-ReindexIndex(RangeVar *indexRelation, int options, bool isTopLevel)
+static void
+ReindexIndex(RangeVar *indexRelation, ReindexParams *params, bool isTopLevel)
 {
 	struct ReindexIndexCallbackState state;
 	Oid			indOid;
@@ -2509,10 +2555,10 @@ ReindexIndex(RangeVar *indexRelation, int options, bool isTopLevel)
 	 * upgrade the lock, but that's OK, because other sessions can't hold
 	 * locks on our temporary table.
 	 */
-	state.options = options;
+	state.params = params;
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  (options & REINDEXOPT_CONCURRENTLY) != 0 ?
+									  (params->options & REINDEXOPT_CONCURRENTLY) != 0 ?
 									  ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
@@ -2526,13 +2572,13 @@ ReindexIndex(RangeVar *indexRelation, int options, bool isTopLevel)
 	relkind = get_rel_relkind(indOid);
 
 	if (relkind == RELKIND_PARTITIONED_INDEX)
-		ReindexPartitions(indOid, options, isTopLevel);
-	else if ((options & REINDEXOPT_CONCURRENTLY) != 0 &&
+		ReindexPartitions(indOid, params, isTopLevel);
+	else if ((params->options & REINDEXOPT_CONCURRENTLY) != 0 &&
 			 persistence != RELPERSISTENCE_TEMP)
-		ReindexRelationConcurrently(indOid, options);
+		ReindexRelationConcurrently(indOid, params);
 	else
 		reindex_index(indOid, false, persistence,
-					  options | REINDEXOPT_REPORT_PROGRESS);
+					  params->options | REINDEXOPT_REPORT_PROGRESS);
 }
 
 /*
@@ -2553,7 +2599,7 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
 	 * non-concurrent case and table locks used by index_concurrently_*() for
 	 * concurrent case.
 	 */
-	table_lockmode = ((state->options & REINDEXOPT_CONCURRENTLY) != 0) ?
+	table_lockmode = (state->params->options & REINDEXOPT_CONCURRENTLY) != 0 ?
 		ShareUpdateExclusiveLock : ShareLock;
 
 	/*
@@ -2610,8 +2656,8 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
  * ReindexTable
  *		Recreate all indexes of a table (and of its toast table, if any)
  */
-Oid
-ReindexTable(RangeVar *relation, int options, bool isTopLevel)
+static Oid
+ReindexTable(RangeVar *relation, ReindexParams *params, bool isTopLevel)
 {
 	Oid			heapOid;
 	bool		result;
@@ -2625,17 +2671,17 @@ ReindexTable(RangeVar *relation, int options, bool isTopLevel)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   (options & REINDEXOPT_CONCURRENTLY) != 0 ?
+									   (params->options & REINDEXOPT_CONCURRENTLY) != 0 ?
 									   ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
 	if (get_rel_relkind(heapOid) == RELKIND_PARTITIONED_TABLE)
-		ReindexPartitions(heapOid, options, isTopLevel);
-	else if ((options & REINDEXOPT_CONCURRENTLY) != 0 &&
+		ReindexPartitions(heapOid, params, isTopLevel);
+	else if ((params->options & REINDEXOPT_CONCURRENTLY) != 0 &&
 			 get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
-		result = ReindexRelationConcurrently(heapOid, options);
+		result = ReindexRelationConcurrently(heapOid, params);
 
 		if (!result)
 			ereport(NOTICE,
@@ -2647,7 +2693,7 @@ ReindexTable(RangeVar *relation, int options, bool isTopLevel)
 		result = reindex_relation(heapOid,
 								  REINDEX_REL_PROCESS_TOAST |
 								  REINDEX_REL_CHECK_CONSTRAINTS,
-								  options | REINDEXOPT_REPORT_PROGRESS);
+								  params->options | REINDEXOPT_REPORT_PROGRESS);
 		if (!result)
 			ereport(NOTICE,
 					(errmsg("table \"%s\" has no indexes to reindex",
@@ -2665,9 +2711,9 @@ ReindexTable(RangeVar *relation, int options, bool isTopLevel)
  * separate transaction, so we can release the lock on it right away.
  * That means this must not be called within a user transaction block!
  */
-void
+static void
 ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
-					  int options)
+					  ReindexParams *params)
 {
 	Oid			objectOid;
 	Relation	relationRelation;
@@ -2686,7 +2732,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
 	if (objectKind == REINDEX_OBJECT_SYSTEM &&
-		(options & REINDEXOPT_CONCURRENTLY) != 0)
+		(params->options & REINDEXOPT_CONCURRENTLY) != 0)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2794,7 +2840,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
 		 * Skip system tables, since index_create() would reject indexing them
 		 * concurrently (and it would likely fail if we tried).
 		 */
-		if ((options & REINDEXOPT_CONCURRENTLY) != 0 &&
+		if ((params->options & REINDEXOPT_CONCURRENTLY) != 0 &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2829,7 +2875,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
 	 * Process each relation listed in a separate transaction.  Note that this
 	 * commits and then starts a new transaction immediately.
 	 */
-	ReindexMultipleInternal(relids, options);
+	ReindexMultipleInternal(relids, params);
 
 	MemoryContextDelete(private_context);
 }
@@ -2860,7 +2906,7 @@ reindex_error_callback(void *arg)
  * by the caller.
  */
 static void
-ReindexPartitions(Oid relid, int options, bool isTopLevel)
+ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
 {
 	List	   *partitions = NIL;
 	char		relkind = get_rel_relkind(relid);
@@ -2937,7 +2983,7 @@ ReindexPartitions(Oid relid, int options, bool isTopLevel)
 	 * Process each partition listed in a separate transaction.  Note that
 	 * this commits and then starts a new transaction immediately.
 	 */
-	ReindexMultipleInternal(partitions, options);
+	ReindexMultipleInternal(partitions, params);
 
 	/*
 	 * Clean up working storage --- note we must do this after
@@ -2955,7 +3001,7 @@ ReindexPartitions(Oid relid, int options, bool isTopLevel)
  * and starts a new transaction when finished.
  */
 static void
-ReindexMultipleInternal(List *relids, int options)
+ReindexMultipleInternal(List *relids, ReindexParams *params)
 {
 	ListCell   *l;
 
@@ -2991,18 +3037,17 @@ ReindexMultipleInternal(List *relids, int options)
 		Assert(relkind != RELKIND_PARTITIONED_INDEX &&
 			   relkind != RELKIND_PARTITIONED_TABLE);
 
-		if ((options & REINDEXOPT_CONCURRENTLY) != 0 &&
+		if ((params->options & REINDEXOPT_CONCURRENTLY) != 0 &&
 			relpersistence != RELPERSISTENCE_TEMP)
 		{
-			(void) ReindexRelationConcurrently(relid,
-											   options |
-											   REINDEXOPT_MISSING_OK);
+			params->options |= REINDEXOPT_MISSING_OK;
+			(void) ReindexRelationConcurrently(relid, params);
 			/* ReindexRelationConcurrently() does the verbose output */
 		}
 		else if (relkind == RELKIND_INDEX)
 		{
 			reindex_index(relid, false, relpersistence,
-						  options |
+						  params->options |
 						  REINDEXOPT_REPORT_PROGRESS |
 						  REINDEXOPT_MISSING_OK);
 			PopActiveSnapshot();
@@ -3015,11 +3060,11 @@ ReindexMultipleInternal(List *relids, int options)
 			result = reindex_relation(relid,
 									  REINDEX_REL_PROCESS_TOAST |
 									  REINDEX_REL_CHECK_CONSTRAINTS,
-									  options |
+									  params->options |
 									  REINDEXOPT_REPORT_PROGRESS |
 									  REINDEXOPT_MISSING_OK);
 
-			if (result && (options & REINDEXOPT_VERBOSE))
+			if (result && params->options & REINDEXOPT_VERBOSE)
 				ereport(INFO,
 						(errmsg("table \"%s.%s\" was reindexed",
 								get_namespace_name(get_rel_namespace(relid)),
@@ -3059,7 +3104,7 @@ ReindexMultipleInternal(List *relids, int options)
  * anyway, and a non-concurrent reindex is more efficient.
  */
 static bool
-ReindexRelationConcurrently(Oid relationOid, int options)
+ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 {
 	List	   *heapRelationIds = NIL;
 	List	   *indexIds = NIL;
@@ -3092,7 +3137,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
 											"ReindexConcurrent",
 											ALLOCSET_SMALL_SIZES);
 
-	if (options & REINDEXOPT_VERBOSE)
+	if (params->options & REINDEXOPT_VERBOSE)
 	{
 		/* Save data needed by REINDEX VERBOSE in private context */
 		oldcontext = MemoryContextSwitchTo(private_context);
@@ -3137,7 +3182,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
 							 errmsg("cannot reindex system catalogs concurrently")));
 
 				/* Open relation to get its indexes */
-				if ((options & REINDEXOPT_MISSING_OK) != 0)
+				if (params->options & REINDEXOPT_MISSING_OK)
 				{
 					heapRelation = try_table_open(relationOid,
 												  ShareUpdateExclusiveLock);
@@ -3233,7 +3278,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
 		case RELKIND_INDEX:
 			{
 				Oid			heapId = IndexGetRelation(relationOid,
-													  (options & REINDEXOPT_MISSING_OK) != 0);
+													  (params->options & REINDEXOPT_MISSING_OK) != 0);
 				Relation	heapRelation;
 
 				/* if relation is missing, leave */
@@ -3262,7 +3307,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
 				 * to rebuild is not complete yet, and REINDEXOPT_MISSING_OK
 				 * should not be used once all the session locks are taken.
 				 */
-				if ((options & REINDEXOPT_MISSING_OK) != 0)
+				if ((params->options & REINDEXOPT_MISSING_OK) != 0)
 				{
 					heapRelation = try_table_open(heapId,
 												  ShareUpdateExclusiveLock);
@@ -3754,7 +3799,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
 	StartTransactionCommand();
 
 	/* Log what we did */
-	if (options & REINDEXOPT_VERBOSE)
+	if (params->options & REINDEXOPT_VERBOSE)
 	{
 		if (relkind == RELKIND_INDEX)
 			ereport(INFO,
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index a42ead7d69..3991a834b4 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -917,45 +917,7 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 			break;
 
 		case T_ReindexStmt:
-			{
-				ReindexStmt *stmt = (ReindexStmt *) parsetree;
-				int			options;
-
-				options = ReindexParseOptions(pstate, stmt);
-				if ((options & REINDEXOPT_CONCURRENTLY) != 0)
-					PreventInTransactionBlock(isTopLevel,
-											  "REINDEX CONCURRENTLY");
-
-				switch (stmt->kind)
-				{
-					case REINDEX_OBJECT_INDEX:
-						ReindexIndex(stmt->relation, options, isTopLevel);
-						break;
-					case REINDEX_OBJECT_TABLE:
-						ReindexTable(stmt->relation, options, isTopLevel);
-						break;
-					case REINDEX_OBJECT_SCHEMA:
-					case REINDEX_OBJECT_SYSTEM:
-					case REINDEX_OBJECT_DATABASE:
-
-						/*
-						 * This cannot run inside a user transaction block; if
-						 * we were inside a transaction, then its commit- and
-						 * start-transaction-command calls would not have the
-						 * intended effect!
-						 */
-						PreventInTransactionBlock(isTopLevel,
-												  (stmt->kind == REINDEX_OBJECT_SCHEMA) ? "REINDEX SCHEMA" :
-												  (stmt->kind == REINDEX_OBJECT_SYSTEM) ? "REINDEX SYSTEM" :
-												  "REINDEX DATABASE");
-						ReindexMultipleTables(stmt->name, stmt->kind, options);
-						break;
-					default:
-						elog(ERROR, "unrecognized object type: %d",
-							 (int) stmt->kind);
-						break;
-				}
-			}
+			ExecReindex(pstate, (ReindexStmt *)parsetree, isTopLevel);
 			break;
 
 			/*
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index 1133ae1143..621d5919a3 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -34,11 +34,8 @@ extern ObjectAddress DefineIndex(Oid relationId,
 								 bool check_not_in_use,
 								 bool skip_build,
 								 bool quiet);
-extern int	ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt);
-extern void ReindexIndex(RangeVar *indexRelation, int options, bool isTopLevel);
-extern Oid	ReindexTable(RangeVar *relation, int options, bool isTopLevel);
-extern void ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
-								  int options);
+extern void ExecReindex(ParseState *pstate, ReindexStmt *stmt,
+						bool isTopLevel);
 extern char *makeObjectName(const char *name1, const char *name2,
 							const char *label);
 extern char *ChooseRelationName(const char *name1, const char *name2,
-- 
2.17.0

--8jEihaNHb65WmIJG
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v32-0002-Allow-REINDEX-to-change-tablespace.patch"