Simplify index_[constraint_]create API

Instead of passing large swaths of boolean arguments, define some flags
that can be used in a bitmask.  This makes it easier not only to figure
out what each call site is doing, but also to add some new flags.

The flags are split in two -- one set for index_create directly and
another for constraints.  index_create() itself receives both, and then
passes down the latter to index_constraint_create(), which can also be
called standalone.

Discussion: https://postgr.es/m/20171023151251.j75uoe27gajdjmlm@alvherre.pgsql
Reviewed-by: Simon Riggs

src/backend/catalog/index.c

@@ -680,19 +680,25 @@ UpdateIndexRelation(Oid indexoid,
  * classObjectId: array of index opclass OIDs, one per index column
  * coloptions: array of per-index-column indoption settings
  * reloptions: AM-specific options
- * isprimary: index is a PRIMARY KEY
- * isconstraint: index is owned by PRIMARY KEY, UNIQUE, or EXCLUSION constraint
- * deferrable: constraint is DEFERRABLE
- * initdeferred: constraint is INITIALLY DEFERRED
- * allow_system_table_mods: allow table to be a system catalog
- * skip_build: true to skip the index_build() step for the moment; caller
- *		must do it later (typically via reindex_index())
- * concurrent: if true, do not lock the table against writers.  The index
- *		will be marked "invalid" and the caller must take additional steps
+ * flags: bitmask that can include any combination of these bits:
+ *		INDEX_CREATE_IS_PRIMARY
+ *			the index is a primary key
+ *		INDEX_CREATE_ADD_CONSTRAINT:
+ *			invoke index_constraint_create also
+ *		INDEX_CREATE_SKIP_BUILD:
+ *			skip the index_build() step for the moment; caller must do it
+ *			later (typically via reindex_index())
+ *		INDEX_CREATE_CONCURRENT:
+ *			do not lock the table against writers.  The index will be
+ *			marked "invalid" and the caller must take additional steps
  *			to fix it up.
+ *		INDEX_CREATE_IF_NOT_EXISTS:
+ *			do not throw an error if a relation with the same name
+ *			already exists.
+ * constr_flags: flags passed to index_constraint_create
+ *		(only if INDEX_CREATE_ADD_CONSTRAINT is set)
+ * allow_system_table_mods: allow table to be a system catalog
  * is_internal: if true, post creation hook for new index
- * if_not_exists: if true, do not throw an error if a relation with
- *		the same name already exists.
  *
  * Returns the OID of the created index.
  */
@@ -709,15 +715,10 @@ index_create(Relation heapRelation,
 			 Oid *classObjectId,
 			 int16 *coloptions,
 			 Datum reloptions,
-			 bool isprimary,
-			 bool isconstraint,
-			 bool deferrable,
-			 bool initdeferred,
+			 bits16 flags,
+			 bits16 constr_flags,
 			 bool allow_system_table_mods,
-			 bool skip_build,
-			 bool concurrent,
-			 bool is_internal,
-			 bool if_not_exists)
+			 bool is_internal)
 {
 	Oid			heapRelationId = RelationGetRelid(heapRelation);
 	Relation	pg_class;
@@ -729,6 +730,12 @@ index_create(Relation heapRelation,
 	Oid			namespaceId;
 	int			i;
 	char		relpersistence;
+	bool		isprimary = (flags & INDEX_CREATE_IS_PRIMARY) != 0;
+	bool		concurrent = (flags & INDEX_CREATE_CONCURRENT) != 0;
+
+	/* constraint flags can only be set when a constraint is requested */
+	Assert((constr_flags == 0) ||
+		   ((flags & INDEX_CREATE_ADD_CONSTRAINT) != 0));
 
 	is_exclusion = (indexInfo->ii_ExclusionOps != NULL);
 
@@ -794,7 +801,7 @@ index_create(Relation heapRelation,
 
 	if (get_relname_relid(indexRelationName, namespaceId))
 	{
-		if (if_not_exists)
+		if ((flags & INDEX_CREATE_IF_NOT_EXISTS) != 0)
 		{
 			ereport(NOTICE,
 					(errcode(ERRCODE_DUPLICATE_TABLE),
@@ -917,7 +924,7 @@ index_create(Relation heapRelation,
 	UpdateIndexRelation(indexRelationId, heapRelationId, indexInfo,
 						collationObjectId, classObjectId, coloptions,
 						isprimary, is_exclusion,
-						!deferrable,
+						(constr_flags & INDEX_CONSTR_CREATE_DEFERRABLE) == 0,
 						!concurrent);
 
 	/*
@@ -943,7 +950,7 @@ index_create(Relation heapRelation,
 		myself.objectId = indexRelationId;
 		myself.objectSubId = 0;
 
-		if (isconstraint)
+		if ((flags & INDEX_CREATE_ADD_CONSTRAINT) != 0)
 		{
 			char		constraintType;
 
@@ -964,11 +971,7 @@ index_create(Relation heapRelation,
 									indexInfo,
 									indexRelationName,
 									constraintType,
-									deferrable,
-									initdeferred,
-									false,	/* already marked primary */
-									false,	/* pg_index entry is OK */
-									false,	/* no old dependencies */
+									constr_flags,
 									allow_system_table_mods,
 									is_internal);
 		}
@@ -1005,10 +1008,6 @@ index_create(Relation heapRelation,
 
 				recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO);
 			}
-
-			/* Non-constraint indexes can't be deferrable */
-			Assert(!deferrable);
-			Assert(!initdeferred);
 		}
 
 		/* Store dependency on collations */
@@ -1059,9 +1058,7 @@ index_create(Relation heapRelation,
 	else
 	{
 		/* Bootstrap mode - assert we weren't asked for constraint support */
-		Assert(!isconstraint);
-		Assert(!deferrable);
-		Assert(!initdeferred);
+		Assert((flags & INDEX_CREATE_ADD_CONSTRAINT) == 0);
 	}
 
 	/* Post creation hook for new index */
@@ -1089,15 +1086,16 @@ index_create(Relation heapRelation,
 	 * If this is bootstrap (initdb) time, then we don't actually fill in the
 	 * index yet.  We'll be creating more indexes and classes later, so we
 	 * delay filling them in until just before we're done with bootstrapping.
-	 * Similarly, if the caller specified skip_build then filling the index is
-	 * delayed till later (ALTER TABLE can save work in some cases with this).
-	 * Otherwise, we call the AM routine that constructs the index.
+	 * Similarly, if the caller specified to skip the build then filling the
+	 * index is delayed till later (ALTER TABLE can save work in some cases
+	 * with this).  Otherwise, we call the AM routine that constructs the
+	 * index.
 	 */
 	if (IsBootstrapProcessingMode())
 	{
 		index_register(heapRelationId, indexRelationId, indexInfo);
 	}
-	else if (skip_build)
+	else if ((flags & INDEX_CREATE_SKIP_BUILD) != 0)
 	{
 		/*
 		 * Caller is responsible for filling the index later on.  However,
@@ -1137,12 +1135,13 @@ index_create(Relation heapRelation,
  * constraintName: what it say (generally, should match name of index)
  * constraintType: one of CONSTRAINT_PRIMARY, CONSTRAINT_UNIQUE, or
  *		CONSTRAINT_EXCLUSION
- * deferrable: constraint is DEFERRABLE
- * initdeferred: constraint is INITIALLY DEFERRED
- * mark_as_primary: if true, set flags to mark index as primary key
- * update_pgindex: if true, update pg_index row (else caller's done that)
- * remove_old_dependencies: if true, remove existing dependencies of index
- *		on table's columns
+ * flags: bitmask that can include any combination of these bits:
+ *		INDEX_CONSTR_CREATE_MARK_AS_PRIMARY: index is a PRIMARY KEY
+ *		INDEX_CONSTR_CREATE_DEFERRABLE: constraint is DEFERRABLE
+ *		INDEX_CONSTR_CREATE_INIT_DEFERRED: constraint is INITIALLY DEFERRED
+ *		INDEX_CONSTR_CREATE_UPDATE_INDEX: update the pg_index row
+ *		INDEX_CONSTR_CREATE_REMOVE_OLD_DEPS: remove existing dependencies
+ *			of index on table's columns
  * allow_system_table_mods: allow table to be a system catalog
  * is_internal: index is constructed due to internal process
  */
@@ -1152,11 +1151,7 @@ index_constraint_create(Relation heapRelation,
 						IndexInfo *indexInfo,
 						const char *constraintName,
 						char constraintType,
-						bool deferrable,
-						bool initdeferred,
-						bool mark_as_primary,
-						bool update_pgindex,
-						bool remove_old_dependencies,
+						bits16 constr_flags,
 						bool allow_system_table_mods,
 						bool is_internal)
 {
@@ -1164,6 +1159,13 @@ index_constraint_create(Relation heapRelation,
 	ObjectAddress myself,
 				referenced;
 	Oid			conOid;
+	bool		deferrable;
+	bool		initdeferred;
+	bool		mark_as_primary;
+
+	deferrable = (constr_flags & INDEX_CONSTR_CREATE_DEFERRABLE) != 0;
+	initdeferred = (constr_flags & INDEX_CONSTR_CREATE_INIT_DEFERRED) != 0;
+	mark_as_primary = (constr_flags & INDEX_CONSTR_CREATE_MARK_AS_PRIMARY) != 0;
 
 	/* constraint creation support doesn't work while bootstrapping */
 	Assert(!IsBootstrapProcessingMode());
@@ -1190,7 +1192,7 @@ index_constraint_create(Relation heapRelation,
 	 * has any expressions or predicate, but we'd never be turning such an
 	 * index into a UNIQUE or PRIMARY KEY constraint.
 	 */
-	if (remove_old_dependencies)
+	if (constr_flags & INDEX_CONSTR_CREATE_REMOVE_OLD_DEPS)
 		deleteDependencyRecordsForClass(RelationRelationId, indexRelationId,
 										RelationRelationId, DEPENDENCY_AUTO);
 
@@ -1295,7 +1297,8 @@ index_constraint_create(Relation heapRelation,
 	 * is a risk that concurrent readers of the table will miss seeing this
 	 * index at all.
 	 */
-	if (update_pgindex && (mark_as_primary || deferrable))
+	if ((constr_flags & INDEX_CONSTR_CREATE_UPDATE_INDEX) &&
+		(mark_as_primary || deferrable))
 	{
 		Relation	pg_index;
 		HeapTuple	indexTuple;

src/backend/catalog/toasting.c

@@ -333,8 +333,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
 				 BTREE_AM_OID,
 				 rel->rd_rel->reltablespace,
 				 collationObjectId, classObjectId, coloptions, (Datum) 0,
-				 true, false, false, false,
-				 true, false, false, true, false);
+				 INDEX_CREATE_IS_PRIMARY, 0, true, true);
 
 	heap_close(toast_rel, NoLock);
 

src/backend/commands/indexcmds.c

@@ -333,6 +333,8 @@ DefineIndex(Oid relationId,
 	Datum		reloptions;
 	int16	   *coloptions;
 	IndexInfo  *indexInfo;
+	bits16		flags;
+	bits16		constr_flags;
 	int			numberOfAttributes;
 	TransactionId limitXmin;
 	VirtualTransactionId *old_snapshots;
@@ -661,20 +663,35 @@ DefineIndex(Oid relationId,
 	Assert(!OidIsValid(stmt->oldNode) || (skip_build && !stmt->concurrent));
 
 	/*
-	 * Make the catalog entries for the index, including constraints. Then, if
-	 * not skip_build || concurrent, actually build the index.
+	 * Make the catalog entries for the index, including constraints. This
+	 * step also actually builds the index, except if caller requested not to
+	 * or in concurrent mode, in which case it'll be done later.
 	 */
+	flags = constr_flags = 0;
+	if (stmt->isconstraint)
+		flags |= INDEX_CREATE_ADD_CONSTRAINT;
+	if (skip_build || stmt->concurrent)
+		flags |= INDEX_CREATE_SKIP_BUILD;
+	if (stmt->if_not_exists)
+		flags |= INDEX_CREATE_IF_NOT_EXISTS;
+	if (stmt->concurrent)
+		flags |= INDEX_CREATE_CONCURRENT;
+	if (stmt->primary)
+		flags |= INDEX_CREATE_IS_PRIMARY;
+
+	if (stmt->deferrable)
+		constr_flags |= INDEX_CONSTR_CREATE_DEFERRABLE;
+	if (stmt->initdeferred)
+		constr_flags |= INDEX_CONSTR_CREATE_INIT_DEFERRED;
+
 	indexRelationId =
 		index_create(rel, indexRelationName, indexRelationId, stmt->oldNode,
 					 indexInfo, indexColNames,
 					 accessMethodId, tablespaceId,
 					 collationObjectId, classObjectId,
-					 coloptions, reloptions, stmt->primary,
-					 stmt->isconstraint, stmt->deferrable, stmt->initdeferred,
-					 allowSystemTableMods,
-					 skip_build || stmt->concurrent,
-					 stmt->concurrent, !check_rights,
-					 stmt->if_not_exists);
+					 coloptions, reloptions,
+					 flags, constr_flags,
+					 allowSystemTableMods, !check_rights);
 
 	ObjectAddressSet(address, RelationRelationId, indexRelationId);
 

src/backend/commands/tablecmds.c

@@ -6836,6 +6836,7 @@ ATExecAddIndexConstraint(AlteredTableInfo *tab, Relation rel,
 	char	   *constraintName;
 	char		constraintType;
 	ObjectAddress address;
+	bits16		flags;
 
 	Assert(IsA(stmt, IndexStmt));
 	Assert(OidIsValid(index_oid));
@@ -6880,16 +6881,18 @@ ATExecAddIndexConstraint(AlteredTableInfo *tab, Relation rel,
 		constraintType = CONSTRAINT_UNIQUE;
 
 	/* Create the catalog entries for the constraint */
+	flags = INDEX_CONSTR_CREATE_UPDATE_INDEX |
+		INDEX_CONSTR_CREATE_REMOVE_OLD_DEPS |
+		(stmt->initdeferred ? INDEX_CONSTR_CREATE_INIT_DEFERRED : 0) |
+		(stmt->deferrable ? INDEX_CONSTR_CREATE_DEFERRABLE : 0) |
+		(stmt->primary ? INDEX_CONSTR_CREATE_MARK_AS_PRIMARY : 0);
+
 	address = index_constraint_create(rel,
 									  index_oid,
 									  indexInfo,
 									  constraintName,
 									  constraintType,
-									  stmt->deferrable,
-									  stmt->initdeferred,
-									  stmt->primary,
-									  true, /* update pg_index */
-									  true, /* remove old dependencies */
+									  flags,
 									  allowSystemTableMods,
 									  false);	/* is_internal */
 

src/include/catalog/index.h

@@ -42,6 +42,12 @@ extern void index_check_primary_key(Relation heapRel,
 						IndexInfo *indexInfo,
 						bool is_alter_table);
 
+#define	INDEX_CREATE_IS_PRIMARY				(1 << 0)
+#define	INDEX_CREATE_ADD_CONSTRAINT			(1 << 1)
+#define	INDEX_CREATE_SKIP_BUILD				(1 << 2)
+#define	INDEX_CREATE_CONCURRENT				(1 << 3)
+#define	INDEX_CREATE_IF_NOT_EXISTS			(1 << 4)
+
 extern Oid index_create(Relation heapRelation,
 			 const char *indexRelationName,
 			 Oid indexRelationId,
@@ -54,26 +60,23 @@ extern Oid index_create(Relation heapRelation,
 			 Oid *classObjectId,
 			 int16 *coloptions,
 			 Datum reloptions,
-			 bool isprimary,
-			 bool isconstraint,
-			 bool deferrable,
-			 bool initdeferred,
+			 bits16 flags,
+			 bits16 constr_flags,
 			 bool allow_system_table_mods,
-			 bool skip_build,
-			 bool concurrent,
-			 bool is_internal,
-			 bool if_not_exists);
+			 bool is_internal);
+
+#define	INDEX_CONSTR_CREATE_MARK_AS_PRIMARY	(1 << 0)
+#define	INDEX_CONSTR_CREATE_DEFERRABLE		(1 << 1)
+#define	INDEX_CONSTR_CREATE_INIT_DEFERRED	(1 << 2)
+#define	INDEX_CONSTR_CREATE_UPDATE_INDEX	(1 << 3)
+#define	INDEX_CONSTR_CREATE_REMOVE_OLD_DEPS	(1 << 4)
 
 extern ObjectAddress index_constraint_create(Relation heapRelation,
 						Oid indexRelationId,
 						IndexInfo *indexInfo,
 						const char *constraintName,
 						char constraintType,
-						bool deferrable,
-						bool initdeferred,
-						bool mark_as_primary,
-						bool update_pgindex,
-						bool remove_old_dependencies,
+						bits16 constr_flags,
 						bool allow_system_table_mods,
 						bool is_internal);