Fix CatalogTupleInsert/Update abstraction for case of shared indstate.

Add CatalogTupleInsertWithInfo and CatalogTupleUpdateWithInfo to let
callers use the CatalogTupleXXX abstraction layer even in cases where
we want to share the results of CatalogOpenIndexes across multiple
inserts/updates for efficiency.  This finishes the job begun in commit
2f5c9d9c, by allowing some remaining simple_heap_insert/update
calls to be replaced.  The abstraction layer is now complete enough
that we don't have to export CatalogIndexInsert at all anymore.

Also, this fixes several places in which 2f5c9d9c introduced performance
regressions by using retail CatalogTupleInsert or CatalogTupleUpdate even
though the previous coding had been able to amortize CatalogOpenIndexes
work across multiple tuples.

A possible future improvement is to arrange for the indexing.c functions
to cache the CatalogIndexState somewhere, maybe in the relcache, in which
case we could get rid of CatalogTupleInsertWithInfo and
CatalogTupleUpdateWithInfo again.  But that's a task for another day.

Discussion: https://postgr.es/m/27502.1485981379@sss.pgh.pa.us

src/backend/catalog/heap.c

@@ -586,9 +586,10 @@ CheckAttributeType(const char *attname,
  * attribute to insert (but we ignore attacl and attoptions, which are always
  * initialized to NULL).
  *
- * indstate is the index state for CatalogIndexInsert.  It can be passed as
- * NULL, in which case we'll fetch the necessary info.  (Don't do this when
- * inserting multiple attributes, because it's a tad more expensive.)
+ * indstate is the index state for CatalogTupleInsertWithInfo.  It can be
+ * passed as NULL, in which case we'll fetch the necessary info.  (Don't do
+ * this when inserting multiple attributes, because it's a tad more
+ * expensive.)
  */
 void
 InsertPgAttributeTuple(Relation pg_attribute_rel,
@@ -630,18 +631,10 @@ InsertPgAttributeTuple(Relation pg_attribute_rel,
 	tup = heap_form_tuple(RelationGetDescr(pg_attribute_rel), values, nulls);
 
 	/* finally insert the new tuple, update the indexes, and clean up */
-	simple_heap_insert(pg_attribute_rel, tup);
-
 	if (indstate != NULL)
-		CatalogIndexInsert(indstate, tup);
+		CatalogTupleInsertWithInfo(pg_attribute_rel, tup, indstate);
 	else
-	{
-		CatalogIndexState indstate;
-
-		indstate = CatalogOpenIndexes(pg_attribute_rel);
-		CatalogIndexInsert(indstate, tup);
-		CatalogCloseIndexes(indstate);
-	}
+		CatalogTupleInsert(pg_attribute_rel, tup);
 
 	heap_freetuple(tup);
 }

src/backend/catalog/indexing.c

@@ -68,7 +68,7 @@ CatalogCloseIndexes(CatalogIndexState indstate)
  *
  * This is effectively a cut-down version of ExecInsertIndexTuples.
  */
-void
+static void
 CatalogIndexInsert(CatalogIndexState indstate, HeapTuple heapTuple)
 {
 	int			i;
@@ -148,18 +148,20 @@ CatalogIndexInsert(CatalogIndexState indstate, HeapTuple heapTuple)
 /*
  * CatalogTupleInsert - do heap and indexing work for a new catalog tuple
  *
+ * Insert the tuple data in "tup" into the specified catalog relation.
+ * The Oid of the inserted tuple is returned.
+ *
  * This is a convenience routine for the common case of inserting a single
  * tuple in a system catalog; it inserts a new heap tuple, keeping indexes
- * current.  Avoid using it for multiple tuples, since opening the indexes and
- * building the index info structures is moderately expensive.
- *
- * The Oid of the inserted tuple is returned.
+ * current.  Avoid using it for multiple tuples, since opening the indexes
+ * and building the index info structures is moderately expensive.
+ * (Use CatalogTupleInsertWithInfo in such cases.)
  */
 Oid
 CatalogTupleInsert(Relation heapRel, HeapTuple tup)
 {
 	CatalogIndexState indstate;
-	Oid		oid;
+	Oid			oid;
 
 	indstate = CatalogOpenIndexes(heapRel);
 
@@ -171,14 +173,37 @@ CatalogTupleInsert(Relation heapRel, HeapTuple tup)
 	return oid;
 }
 
+/*
+ * CatalogTupleInsertWithInfo - as above, but with caller-supplied index info
+ *
+ * This should be used when it's important to amortize CatalogOpenIndexes/
+ * CatalogCloseIndexes work across multiple insertions.  At some point we
+ * might cache the CatalogIndexState data somewhere (perhaps in the relcache)
+ * so that callers needn't trouble over this ... but we don't do so today.
+ */
+Oid
+CatalogTupleInsertWithInfo(Relation heapRel, HeapTuple tup,
+						   CatalogIndexState indstate)
+{
+	Oid			oid;
+
+	oid = simple_heap_insert(heapRel, tup);
+
+	CatalogIndexInsert(indstate, tup);
+
+	return oid;
+}
+
 /*
  * CatalogTupleUpdate - do heap and indexing work for updating a catalog tuple
  *
+ * Update the tuple identified by "otid", replacing it with the data in "tup".
+ *
  * This is a convenience routine for the common case of updating a single
- * tuple in a system catalog; it updates one heap tuple (identified by otid)
- * with tup, keeping indexes current.  Avoid using it for multiple tuples,
- * since opening the indexes and building the index info structures is
- * moderately expensive.
+ * tuple in a system catalog; it updates one heap tuple, keeping indexes
+ * current.  Avoid using it for multiple tuples, since opening the indexes
+ * and building the index info structures is moderately expensive.
+ * (Use CatalogTupleUpdateWithInfo in such cases.)
  */
 void
 CatalogTupleUpdate(Relation heapRel, ItemPointer otid, HeapTuple tup)
@@ -193,15 +218,37 @@ CatalogTupleUpdate(Relation heapRel, ItemPointer otid, HeapTuple tup)
 	CatalogCloseIndexes(indstate);
 }
 
+/*
+ * CatalogTupleUpdateWithInfo - as above, but with caller-supplied index info
+ *
+ * This should be used when it's important to amortize CatalogOpenIndexes/
+ * CatalogCloseIndexes work across multiple updates.  At some point we
+ * might cache the CatalogIndexState data somewhere (perhaps in the relcache)
+ * so that callers needn't trouble over this ... but we don't do so today.
+ */
+void
+CatalogTupleUpdateWithInfo(Relation heapRel, ItemPointer otid, HeapTuple tup,
+						   CatalogIndexState indstate)
+{
+	simple_heap_update(heapRel, otid, tup);
+
+	CatalogIndexInsert(indstate, tup);
+}
+
 /*
  * CatalogTupleDelete - do heap and indexing work for deleting a catalog tuple
  *
- * Delete the tuple identified by tid in the specified catalog.
+ * Delete the tuple identified by "tid" in the specified catalog.
  *
  * With Postgres heaps, there is no index work to do at deletion time;
  * cleanup will be done later by VACUUM.  However, callers of this function
  * shouldn't have to know that; we'd like a uniform abstraction for all
  * catalog tuple changes.  Hence, provide this currently-trivial wrapper.
+ *
+ * The abstraction is a bit leaky in that we don't provide an optimized
+ * CatalogTupleDeleteWithInfo version, because there is currently nothing to
+ * optimize.  If we ever need that, rather than touching a lot of call sites,
+ * it might be better to do something about caching CatalogIndexState.
  */
 void
 CatalogTupleDelete(Relation heapRel, ItemPointer tid)

src/backend/catalog/pg_depend.c

@@ -107,13 +107,11 @@ recordMultipleDependencies(const ObjectAddress *depender,
 
 			tup = heap_form_tuple(dependDesc->rd_att, values, nulls);
 
-			simple_heap_insert(dependDesc, tup);
-
-			/* keep indexes current */
+			/* fetch index info only when we know we need it */
 			if (indstate == NULL)
 				indstate = CatalogOpenIndexes(dependDesc);
 
-			CatalogIndexInsert(indstate, tup);
+			CatalogTupleInsertWithInfo(dependDesc, tup, indstate);
 
 			heap_freetuple(tup);
 		}

src/backend/catalog/pg_shdepend.c

@@ -753,7 +753,7 @@ copyTemplateDependencies(Oid templateDbId, Oid newDbId)
 		HeapTuple	newtup;
 
 		newtup = heap_modify_tuple(tup, sdepDesc, values, nulls, replace);
-		CatalogTupleInsert(sdepRel, newtup);
+		CatalogTupleInsertWithInfo(sdepRel, newtup, indstate);
 
 		heap_freetuple(newtup);
 	}

src/backend/commands/cluster.c

@@ -1141,7 +1141,6 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
 				relfilenode2;
 	Oid			swaptemp;
 	char		swptmpchr;
-	CatalogIndexState indstate;
 
 	/* We need writable copies of both pg_class tuples. */
 	relRelation = heap_open(RelationRelationId, RowExclusiveLock);
@@ -1285,13 +1284,13 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
 	 */
 	if (!target_is_pg_class)
 	{
-		simple_heap_update(relRelation, &reltup1->t_self, reltup1);
-		simple_heap_update(relRelation, &reltup2->t_self, reltup2);
+		CatalogIndexState indstate;
 
-		/* Keep system catalogs current */
 		indstate = CatalogOpenIndexes(relRelation);
-		CatalogIndexInsert(indstate, reltup1);
-		CatalogIndexInsert(indstate, reltup2);
+		CatalogTupleUpdateWithInfo(relRelation, &reltup1->t_self, reltup1,
+								   indstate);
+		CatalogTupleUpdateWithInfo(relRelation, &reltup2->t_self, reltup2,
+								   indstate);
 		CatalogCloseIndexes(indstate);
 	}
 	else

src/backend/storage/large_object/inv_api.c

@@ -678,7 +678,8 @@ inv_write(LargeObjectDesc *obj_desc, const char *buf, int nbytes)
 			replace[Anum_pg_largeobject_data - 1] = true;
 			newtup = heap_modify_tuple(oldtuple, RelationGetDescr(lo_heap_r),
 									   values, nulls, replace);
-			CatalogTupleUpdate(lo_heap_r, &newtup->t_self, newtup);
+			CatalogTupleUpdateWithInfo(lo_heap_r, &newtup->t_self, newtup,
+									   indstate);
 			heap_freetuple(newtup);
 
 			/*
@@ -720,7 +721,7 @@ inv_write(LargeObjectDesc *obj_desc, const char *buf, int nbytes)
 			values[Anum_pg_largeobject_pageno - 1] = Int32GetDatum(pageno);
 			values[Anum_pg_largeobject_data - 1] = PointerGetDatum(&workbuf);
 			newtup = heap_form_tuple(lo_heap_r->rd_att, values, nulls);
-			CatalogTupleInsert(lo_heap_r, newtup);
+			CatalogTupleInsertWithInfo(lo_heap_r, newtup, indstate);
 			heap_freetuple(newtup);
 		}
 		pageno++;
@@ -848,7 +849,8 @@ inv_truncate(LargeObjectDesc *obj_desc, int64 len)
 		replace[Anum_pg_largeobject_data - 1] = true;
 		newtup = heap_modify_tuple(oldtuple, RelationGetDescr(lo_heap_r),
 								   values, nulls, replace);
-		CatalogTupleUpdate(lo_heap_r, &newtup->t_self, newtup);
+		CatalogTupleUpdateWithInfo(lo_heap_r, &newtup->t_self, newtup,
+								   indstate);
 		heap_freetuple(newtup);
 	}
 	else
@@ -885,7 +887,7 @@ inv_truncate(LargeObjectDesc *obj_desc, int64 len)
 		values[Anum_pg_largeobject_pageno - 1] = Int32GetDatum(pageno);
 		values[Anum_pg_largeobject_data - 1] = PointerGetDatum(&workbuf);
 		newtup = heap_form_tuple(lo_heap_r->rd_att, values, nulls);
-		CatalogTupleInsert(lo_heap_r, newtup);
+		CatalogTupleInsertWithInfo(lo_heap_r, newtup, indstate);
 		heap_freetuple(newtup);
 	}
 

src/include/catalog/indexing.h

@@ -30,11 +30,14 @@ typedef struct ResultRelInfo *CatalogIndexState;
  */
 extern CatalogIndexState CatalogOpenIndexes(Relation heapRel);
 extern void CatalogCloseIndexes(CatalogIndexState indstate);
-extern void CatalogIndexInsert(CatalogIndexState indstate,
-				   HeapTuple heapTuple);
-extern Oid CatalogTupleInsert(Relation heapRel, HeapTuple tup);
+extern Oid	CatalogTupleInsert(Relation heapRel, HeapTuple tup);
+extern Oid CatalogTupleInsertWithInfo(Relation heapRel, HeapTuple tup,
+						   CatalogIndexState indstate);
 extern void CatalogTupleUpdate(Relation heapRel, ItemPointer otid,
 				   HeapTuple tup);
+extern void CatalogTupleUpdateWithInfo(Relation heapRel,
+						   ItemPointer otid, HeapTuple tup,
+						   CatalogIndexState indstate);
 extern void CatalogTupleDelete(Relation heapRel, ItemPointer tid);