0001-Convert-reindex-options-to-struct.patch

From d3125b3bfb8a3dc23e38f38bcf850ca6fc36f492 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Sat, 12 Dec 2020 09:17:55 +0100
Subject: [PATCH] Convert reindex options to struct

---
 src/backend/catalog/index.c      | 19 ++++---
 src/backend/commands/cluster.c   |  2 +-
 src/backend/commands/indexcmds.c | 94 ++++++++++++++++----------------
 src/backend/commands/tablecmds.c |  2 +-
 src/backend/tcop/utility.c       |  4 +-
 src/include/catalog/index.h      | 16 +++---
 src/include/commands/defrem.h    |  9 +--
 7 files changed, 74 insertions(+), 72 deletions(-)

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 731610c701..06342fddf1 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3594,7 +3594,7 @@ IndexGetRelation(Oid indexId, bool missing_ok)
  */
 void
 reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
-			  int options)
+			  ReindexOptions options)
 {
 	Relation	iRel,
 				heapRelation;
@@ -3602,7 +3602,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 	IndexInfo  *indexInfo;
 	volatile bool skipped_constraint = false;
 	PGRUsage	ru0;
-	bool		progress = (options & REINDEXOPT_REPORT_PROGRESS) != 0;
+	bool		progress = options.REINDEXOPT_REPORT_PROGRESS;
 
 	pg_rusage_init(&ru0);
 
@@ -3611,12 +3611,12 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 	 * we only need to be sure no schema or data changes are going on.
 	 */
 	heapId = IndexGetRelation(indexId,
-							  (options & REINDEXOPT_MISSING_OK) != 0);
+							  options.REINDEXOPT_MISSING_OK);
 	/* if relation is missing, leave */
 	if (!OidIsValid(heapId))
 		return;
 
-	if ((options & REINDEXOPT_MISSING_OK) != 0)
+	if (options.REINDEXOPT_MISSING_OK)
 		heapRelation = try_table_open(heapId, ShareLock);
 	else
 		heapRelation = table_open(heapId, ShareLock);
@@ -3792,7 +3792,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
 	}
 
 	/* Log what we did */
-	if (options & REINDEXOPT_VERBOSE)
+	if (options.REINDEXOPT_VERBOSE)
 		ereport(INFO,
 				(errmsg("index \"%s\" was reindexed",
 						get_rel_name(indexId)),
@@ -3846,7 +3846,7 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
  * index rebuild.
  */
 bool
-reindex_relation(Oid relid, int flags, int options)
+reindex_relation(Oid relid, int flags, ReindexOptions options)
 {
 	Relation	rel;
 	Oid			toast_relid;
@@ -3861,7 +3861,7 @@ reindex_relation(Oid relid, int flags, int options)
 	 * to prevent schema and data changes in it.  The lock level used here
 	 * should match ReindexTable().
 	 */
-	if ((options & REINDEXOPT_MISSING_OK) != 0)
+	if (options.REINDEXOPT_MISSING_OK)
 		rel = try_table_open(relid, ShareLock);
 	else
 		rel = table_open(relid, ShareLock);
@@ -3965,8 +3965,9 @@ reindex_relation(Oid relid, int flags, int options)
 		 * Note that this should fail if the toast relation is missing, so
 		 * reset REINDEXOPT_MISSING_OK.
 		 */
-		result |= reindex_relation(toast_relid, flags,
-								   options & ~(REINDEXOPT_MISSING_OK));
+		ReindexOptions newoptions = options;
+		newoptions.REINDEXOPT_MISSING_OK = false;
+		result |= reindex_relation(toast_relid, flags, newoptions);
 	}
 
 	return result;
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index fd5a6eec86..b0aa3536d1 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -1412,7 +1412,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
 	pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
 								 PROGRESS_CLUSTER_PHASE_REBUILD_INDEX);
 
-	reindex_relation(OIDOldHeap, reindex_flags, 0);
+	reindex_relation(OIDOldHeap, reindex_flags, (ReindexOptions){});
 
 	/* Report that we are now doing clean up */
 	pgstat_progress_update_param(PROGRESS_CLUSTER_PHASE,
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 14d24b3cc4..cd8eaa732d 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -89,9 +89,9 @@ static List *ChooseIndexColumnNames(List *indexElems);
 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, ReindexOptions options, bool isTopLevel);
+static void ReindexMultipleInternal(List *relids, ReindexOptions options);
+static bool ReindexRelationConcurrently(Oid relationOid, ReindexOptions options);
 static void update_relispartition(Oid relationId, bool newval);
 static inline void set_indexsafe_procflags(void);
 
@@ -100,7 +100,7 @@ static inline void set_indexsafe_procflags(void);
  */
 struct ReindexIndexCallbackState
 {
-	int			options;		/* options from statement */
+	ReindexOptions	options;		/* options from statement */
 	Oid			locked_table_oid;	/* tracks previously locked table */
 };
 
@@ -2455,13 +2455,11 @@ ChooseIndexColumnNames(List *indexElems)
  * ReindexParseOptions
  *		Parse list of REINDEX options, returning a bitmask of ReindexOption.
  */
-int
+ReindexOptions
 ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt)
 {
 	ListCell   *lc;
-	int			options = 0;
-	bool		concurrently = false;
-	bool		verbose = false;
+	ReindexOptions options = {};
 
 	/* Parse option list */
 	foreach(lc, stmt->params)
@@ -2469,9 +2467,9 @@ ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt)
 		DefElem    *opt = (DefElem *) lfirst(lc);
 
 		if (strcmp(opt->defname, "verbose") == 0)
-			verbose = defGetBoolean(opt);
+			options.REINDEXOPT_VERBOSE = defGetBoolean(opt);
 		else if (strcmp(opt->defname, "concurrently") == 0)
-			concurrently = defGetBoolean(opt);
+			options.REINDEXOPT_CONCURRENTLY = defGetBoolean(opt);
 		else
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -2480,10 +2478,6 @@ ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt)
 					 parser_errposition(pstate, opt->location)));
 	}
 
-	options =
-		(verbose ? REINDEXOPT_VERBOSE : 0) |
-		(concurrently ? REINDEXOPT_CONCURRENTLY : 0);
-
 	return options;
 }
 
@@ -2492,7 +2486,7 @@ ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt)
  *		Recreate a specific index.
  */
 void
-ReindexIndex(RangeVar *indexRelation, int options, bool isTopLevel)
+ReindexIndex(RangeVar *indexRelation, ReindexOptions options, bool isTopLevel)
 {
 	struct ReindexIndexCallbackState state;
 	Oid			indOid;
@@ -2512,7 +2506,7 @@ ReindexIndex(RangeVar *indexRelation, int options, bool isTopLevel)
 	state.options = options;
 	state.locked_table_oid = InvalidOid;
 	indOid = RangeVarGetRelidExtended(indexRelation,
-									  (options & REINDEXOPT_CONCURRENTLY) != 0 ?
+									  options.REINDEXOPT_CONCURRENTLY ?
 									  ShareUpdateExclusiveLock : AccessExclusiveLock,
 									  0,
 									  RangeVarCallbackForReindexIndex,
@@ -2527,12 +2521,15 @@ ReindexIndex(RangeVar *indexRelation, int options, bool isTopLevel)
 
 	if (relkind == RELKIND_PARTITIONED_INDEX)
 		ReindexPartitions(indOid, options, isTopLevel);
-	else if ((options & REINDEXOPT_CONCURRENTLY) != 0 &&
+	else if (options.REINDEXOPT_CONCURRENTLY &&
 			 persistence != RELPERSISTENCE_TEMP)
 		ReindexRelationConcurrently(indOid, options);
 	else
-		reindex_index(indOid, false, persistence,
-					  options | REINDEXOPT_REPORT_PROGRESS);
+	{
+		ReindexOptions newoptions = options;
+		newoptions.REINDEXOPT_REPORT_PROGRESS = true;
+		reindex_index(indOid, false, persistence, newoptions);
+	}
 }
 
 /*
@@ -2553,7 +2550,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->options.REINDEXOPT_CONCURRENTLY ?
 		ShareUpdateExclusiveLock : ShareLock;
 
 	/*
@@ -2611,7 +2608,7 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
  *		Recreate all indexes of a table (and of its toast table, if any)
  */
 Oid
-ReindexTable(RangeVar *relation, int options, bool isTopLevel)
+ReindexTable(RangeVar *relation, ReindexOptions options, bool isTopLevel)
 {
 	Oid			heapOid;
 	bool		result;
@@ -2625,14 +2622,14 @@ ReindexTable(RangeVar *relation, int options, bool isTopLevel)
 	 * locks on our temporary table.
 	 */
 	heapOid = RangeVarGetRelidExtended(relation,
-									   (options & REINDEXOPT_CONCURRENTLY) != 0 ?
+									   options.REINDEXOPT_CONCURRENTLY ?
 									   ShareUpdateExclusiveLock : ShareLock,
 									   0,
 									   RangeVarCallbackOwnsTable, NULL);
 
 	if (get_rel_relkind(heapOid) == RELKIND_PARTITIONED_TABLE)
 		ReindexPartitions(heapOid, options, isTopLevel);
-	else if ((options & REINDEXOPT_CONCURRENTLY) != 0 &&
+	else if (options.REINDEXOPT_CONCURRENTLY &&
 			 get_rel_persistence(heapOid) != RELPERSISTENCE_TEMP)
 	{
 		result = ReindexRelationConcurrently(heapOid, options);
@@ -2644,10 +2641,12 @@ ReindexTable(RangeVar *relation, int options, bool isTopLevel)
 	}
 	else
 	{
+		ReindexOptions newoptions = options;
+		newoptions.REINDEXOPT_REPORT_PROGRESS = true;
 		result = reindex_relation(heapOid,
 								  REINDEX_REL_PROCESS_TOAST |
 								  REINDEX_REL_CHECK_CONSTRAINTS,
-								  options | REINDEXOPT_REPORT_PROGRESS);
+								  newoptions);
 		if (!result)
 			ereport(NOTICE,
 					(errmsg("table \"%s\" has no indexes to reindex",
@@ -2667,7 +2666,7 @@ ReindexTable(RangeVar *relation, int options, bool isTopLevel)
  */
 void
 ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
-					  int options)
+					  ReindexOptions options)
 {
 	Oid			objectOid;
 	Relation	relationRelation;
@@ -2686,7 +2685,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
 	if (objectKind == REINDEX_OBJECT_SYSTEM &&
-		(options & REINDEXOPT_CONCURRENTLY) != 0)
+		options.REINDEXOPT_CONCURRENTLY)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot reindex system catalogs concurrently")));
@@ -2794,7 +2793,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 (options.REINDEXOPT_CONCURRENTLY &&
 			IsCatalogRelationOid(relid))
 		{
 			if (!concurrent_warning)
@@ -2860,7 +2859,7 @@ reindex_error_callback(void *arg)
  * by the caller.
  */
 static void
-ReindexPartitions(Oid relid, int options, bool isTopLevel)
+ReindexPartitions(Oid relid, ReindexOptions options, bool isTopLevel)
 {
 	List	   *partitions = NIL;
 	char		relkind = get_rel_relkind(relid);
@@ -2955,7 +2954,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, ReindexOptions options)
 {
 	ListCell   *l;
 
@@ -2991,35 +2990,36 @@ ReindexMultipleInternal(List *relids, int options)
 		Assert(relkind != RELKIND_PARTITIONED_INDEX &&
 			   relkind != RELKIND_PARTITIONED_TABLE);
 
-		if ((options & REINDEXOPT_CONCURRENTLY) != 0 &&
+		if (options.REINDEXOPT_CONCURRENTLY &&
 			relpersistence != RELPERSISTENCE_TEMP)
 		{
-			(void) ReindexRelationConcurrently(relid,
-											   options |
-											   REINDEXOPT_MISSING_OK);
+			ReindexOptions newoptions = options;
+			newoptions.REINDEXOPT_MISSING_OK = true;
+			(void) ReindexRelationConcurrently(relid, newoptions);
 			/* ReindexRelationConcurrently() does the verbose output */
 		}
 		else if (relkind == RELKIND_INDEX)
 		{
-			reindex_index(relid, false, relpersistence,
-						  options |
-						  REINDEXOPT_REPORT_PROGRESS |
-						  REINDEXOPT_MISSING_OK);
+			ReindexOptions newoptions = options;
+			newoptions.REINDEXOPT_REPORT_PROGRESS = true;
+			newoptions.REINDEXOPT_MISSING_OK = true;
+			reindex_index(relid, false, relpersistence, newoptions);
 			PopActiveSnapshot();
 			/* reindex_index() does the verbose output */
 		}
 		else
 		{
 			bool		result;
+			ReindexOptions newoptions = options;
+			newoptions.REINDEXOPT_REPORT_PROGRESS = true;
+			newoptions.REINDEXOPT_MISSING_OK = true;
 
 			result = reindex_relation(relid,
 									  REINDEX_REL_PROCESS_TOAST |
 									  REINDEX_REL_CHECK_CONSTRAINTS,
-									  options |
-									  REINDEXOPT_REPORT_PROGRESS |
-									  REINDEXOPT_MISSING_OK);
+									  newoptions);
 
-			if (result && (options & REINDEXOPT_VERBOSE))
+			if (result && options.REINDEXOPT_VERBOSE)
 				ereport(INFO,
 						(errmsg("table \"%s.%s\" was reindexed",
 								get_namespace_name(get_rel_namespace(relid)),
@@ -3059,7 +3059,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, ReindexOptions options)
 {
 	List	   *heapRelationIds = NIL;
 	List	   *indexIds = NIL;
@@ -3092,7 +3092,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
 											"ReindexConcurrent",
 											ALLOCSET_SMALL_SIZES);
 
-	if (options & REINDEXOPT_VERBOSE)
+	if (options.REINDEXOPT_VERBOSE)
 	{
 		/* Save data needed by REINDEX VERBOSE in private context */
 		oldcontext = MemoryContextSwitchTo(private_context);
@@ -3137,7 +3137,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 (options.REINDEXOPT_MISSING_OK)
 				{
 					heapRelation = try_table_open(relationOid,
 												  ShareUpdateExclusiveLock);
@@ -3233,7 +3233,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
 		case RELKIND_INDEX:
 			{
 				Oid			heapId = IndexGetRelation(relationOid,
-													  (options & REINDEXOPT_MISSING_OK) != 0);
+													  options.REINDEXOPT_MISSING_OK);
 				Relation	heapRelation;
 
 				/* if relation is missing, leave */
@@ -3262,7 +3262,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 (options.REINDEXOPT_MISSING_OK)
 				{
 					heapRelation = try_table_open(heapId,
 												  ShareUpdateExclusiveLock);
@@ -3754,7 +3754,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
 	StartTransactionCommand();
 
 	/* Log what we did */
-	if (options & REINDEXOPT_VERBOSE)
+	if (options.REINDEXOPT_VERBOSE)
 	{
 		if (relkind == RELKIND_INDEX)
 			ereport(INFO,
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1fa9f19f08..9f218ac0e4 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1891,7 +1891,7 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
 			/*
 			 * Reconstruct the indexes to match, and we're done.
 			 */
-			reindex_relation(heap_relid, REINDEX_REL_PROCESS_TOAST, 0);
+			reindex_relation(heap_relid, REINDEX_REL_PROCESS_TOAST, (ReindexOptions){});
 		}
 
 		pgstat_count_truncate(rel);
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index a42ead7d69..e5a4e8f662 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -919,10 +919,10 @@ standard_ProcessUtility(PlannedStmt *pstmt,
 		case T_ReindexStmt:
 			{
 				ReindexStmt *stmt = (ReindexStmt *) parsetree;
-				int			options;
+				ReindexOptions options;
 
 				options = ReindexParseOptions(pstate, stmt);
-				if ((options & REINDEXOPT_CONCURRENTLY) != 0)
+				if (options.REINDEXOPT_CONCURRENTLY)
 					PreventInTransactionBlock(isTopLevel,
 											  "REINDEX CONCURRENTLY");
 
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index c041628049..81e3de4d22 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -30,13 +30,13 @@ typedef enum
 } IndexStateFlagsAction;
 
 /* options for REINDEX */
-typedef enum ReindexOption
+typedef struct ReindexOptions
 {
-	REINDEXOPT_VERBOSE = 1 << 0,	/* print progress info */
-	REINDEXOPT_REPORT_PROGRESS = 1 << 1,	/* report pgstat progress */
-	REINDEXOPT_MISSING_OK = 1 << 2, /* skip missing relations */
-	REINDEXOPT_CONCURRENTLY = 1 << 3	/* concurrent mode */
-} ReindexOption;
+	bool REINDEXOPT_VERBOSE;			/* print progress info */
+	bool REINDEXOPT_REPORT_PROGRESS;	/* report pgstat progress */
+	bool REINDEXOPT_MISSING_OK;			/* skip missing relations */
+	bool REINDEXOPT_CONCURRENTLY;		/* concurrent mode */
+} ReindexOptions;
 
 /* state info for validate_index bulkdelete callback */
 typedef struct ValidateIndexState
@@ -146,7 +146,7 @@ extern void index_set_state_flags(Oid indexId, IndexStateFlagsAction action);
 extern Oid	IndexGetRelation(Oid indexId, bool missing_ok);
 
 extern void reindex_index(Oid indexId, bool skip_constraint_checks,
-						  char relpersistence, int options);
+						  char relpersistence, ReindexOptions options);
 
 /* Flag bits for reindex_relation(): */
 #define REINDEX_REL_PROCESS_TOAST			0x01
@@ -155,7 +155,7 @@ extern void reindex_index(Oid indexId, bool skip_constraint_checks,
 #define REINDEX_REL_FORCE_INDEXES_UNLOGGED	0x08
 #define REINDEX_REL_FORCE_INDEXES_PERMANENT 0x10
 
-extern bool reindex_relation(Oid relid, int flags, int options);
+extern bool reindex_relation(Oid relid, int flags, ReindexOptions options);
 
 extern bool ReindexIsProcessingHeap(Oid heapOid);
 extern bool ReindexIsProcessingIndex(Oid indexOid);
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index 1133ae1143..5b1a60d5fa 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -14,6 +14,7 @@
 #ifndef DEFREM_H
 #define DEFREM_H
 
+#include "catalog/index.h"
 #include "catalog/objectaddress.h"
 #include "nodes/params.h"
 #include "parser/parse_node.h"
@@ -34,11 +35,11 @@ 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 ReindexOptions ReindexParseOptions(ParseState *pstate, ReindexStmt *stmt);
+extern void ReindexIndex(RangeVar *indexRelation, ReindexOptions options, bool isTopLevel);
+extern Oid	ReindexTable(RangeVar *relation, ReindexOptions options, bool isTopLevel);
 extern void ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind,
-								  int options);
+								  ReindexOptions options);
 extern char *makeObjectName(const char *name1, const char *name2,
 							const char *label);
 extern char *ChooseRelationName(const char *name1, const char *name2,
-- 
2.29.2