Add missing index_insert_cleanup calls

The optimization for inserts into BRIN indexes added by c1ec02be1d79
relies on a cache that needs to be explicitly released after calling
index_insert(). The commit however failed to invoke the cleanup in
validate_index(), which calls index_insert() indirectly through
table_index_validate_scan().

After inspecting index_insert() callers, it seems unique_key_recheck()
is missing the call too.

Fixed by adding the two missing index_insert_cleanup() calls.

The commit does two additional improvements. The aminsertcleanup()
signature is modified to have the index as the first argument, to make
it more like the other AM callbacks. And the aminsertcleanup() callback
is invoked even if the ii_AmCache is NULL, so that it can decide if the
cleanup is necessary.

Author: Alvaro Herrera, Tomas Vondra
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/202401091043.e3nrqiad6gb7@alvherre.pgsql

8 files changed, 19 insertions, 9 deletions

diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 4f708bba658..bf28400dd84 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -500,11 +500,13 @@ brininsert(Relation idxRel, Datum *values, bool *nulls,
  * Callback to clean up the BrinInsertState once all tuple inserts are done.
  */
 void
-brininsertcleanup(IndexInfo *indexInfo)
+brininsertcleanup(Relation index, IndexInfo *indexInfo)
 {
 	BrinInsertState *bistate = (BrinInsertState *) indexInfo->ii_AmCache;
 
-	Assert(bistate);
+	/* bail out if cache not initialized */
+	if (indexInfo->ii_AmCache == NULL)
+		return;
 
 	/*
 	 * Clean up the revmap. Note that the brinDesc has already been cleaned up
diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c
index 7510159fc8d..dcd04b813d8 100644
--- a/src/backend/access/index/indexam.c
+++ b/src/backend/access/index/indexam.c
@@ -242,10 +242,9 @@ index_insert_cleanup(Relation indexRelation,
 					 IndexInfo *indexInfo)
 {
 	RELATION_CHECKS;
-	Assert(indexInfo);
 
-	if (indexRelation->rd_indam->aminsertcleanup && indexInfo->ii_AmCache)
-		indexRelation->rd_indam->aminsertcleanup(indexInfo);
+	if (indexRelation->rd_indam->aminsertcleanup)
+		indexRelation->rd_indam->aminsertcleanup(indexRelation, indexInfo);
 }
 
 /*
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 9b7ef71d6fe..5a8568c55c9 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3402,6 +3402,9 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
 	/* Done with tuplesort object */
 	tuplesort_end(state.tuplesort);
 
+	/* Make sure to release resources cached in indexInfo (if needed). */
+	index_insert_cleanup(indexRelation, indexInfo);
+
 	elog(DEBUG2,
 		 "validate_index found %.0f heap tuples, %.0f index tuples; inserted %.0f missing tuples",
 		 state.htups, state.itups, state.tups_inserted);
diff --git a/src/backend/commands/constraint.c b/src/backend/commands/constraint.c
index 94d491b7541..f7dc42f7452 100644
--- a/src/backend/commands/constraint.c
+++ b/src/backend/commands/constraint.c
@@ -174,6 +174,9 @@ unique_key_recheck(PG_FUNCTION_ARGS)
 		index_insert(indexRel, values, isnull, &checktid,
 					 trigdata->tg_relation, UNIQUE_CHECK_EXISTING,
 					 false, indexInfo);
+
+		/* Cleanup cache possibly initialized by index_insert. */
+		index_insert_cleanup(indexRel, indexInfo);
 	}
 	else
 	{
diff --git a/src/include/access/amapi.h b/src/include/access/amapi.h
index 00300dd720e..f25c9d58a7d 100644
--- a/src/include/access/amapi.h
+++ b/src/include/access/amapi.h
@@ -114,7 +114,8 @@ typedef bool (*aminsert_function) (Relation indexRelation,
 								   struct IndexInfo *indexInfo);
 
 /* cleanup after insert */
-typedef void (*aminsertcleanup_function) (struct IndexInfo *indexInfo);
+typedef void (*aminsertcleanup_function) (Relation indexRelation,
+										  struct IndexInfo *indexInfo);
 
 /* bulk delete */
 typedef IndexBulkDeleteResult *(*ambulkdelete_function) (IndexVacuumInfo *info,
diff --git a/src/include/access/brin_internal.h b/src/include/access/brin_internal.h
index 1c7eabe6041..a5a9772621c 100644
--- a/src/include/access/brin_internal.h
+++ b/src/include/access/brin_internal.h
@@ -96,7 +96,7 @@ extern bool brininsert(Relation idxRel, Datum *values, bool *nulls,
 					   IndexUniqueCheck checkUnique,
 					   bool indexUnchanged,
 					   struct IndexInfo *indexInfo);
-extern void brininsertcleanup(struct IndexInfo *indexInfo);
+extern void brininsertcleanup(Relation index, struct IndexInfo *indexInfo);
 extern IndexScanDesc brinbeginscan(Relation r, int nkeys, int norderbys);
 extern int64 bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm);
 extern void brinrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
diff --git a/src/test/regress/expected/brin.out b/src/test/regress/expected/brin.out
index 2662bb6ed43..d6779d8c7d2 100644
--- a/src/test/regress/expected/brin.out
+++ b/src/test/regress/expected/brin.out
@@ -575,6 +575,7 @@ DROP TABLE brintest_unlogged;
 -- test that the insert optimization works if no rows end up inserted
 CREATE TABLE brin_insert_optimization (a int);
 INSERT INTO brin_insert_optimization VALUES (1);
-CREATE INDEX ON brin_insert_optimization USING brin (a);
+CREATE INDEX brin_insert_optimization_idx ON brin_insert_optimization USING brin (a);
 UPDATE brin_insert_optimization SET a = a;
+REINDEX INDEX CONCURRENTLY brin_insert_optimization_idx;
 DROP TABLE brin_insert_optimization;
diff --git a/src/test/regress/sql/brin.sql b/src/test/regress/sql/brin.sql
index 0d3beabb3d7..695cfad4bea 100644
--- a/src/test/regress/sql/brin.sql
+++ b/src/test/regress/sql/brin.sql
@@ -519,6 +519,7 @@ DROP TABLE brintest_unlogged;
 -- test that the insert optimization works if no rows end up inserted
 CREATE TABLE brin_insert_optimization (a int);
 INSERT INTO brin_insert_optimization VALUES (1);
-CREATE INDEX ON brin_insert_optimization USING brin (a);
+CREATE INDEX brin_insert_optimization_idx ON brin_insert_optimization USING brin (a);
 UPDATE brin_insert_optimization SET a = a;
+REINDEX INDEX CONCURRENTLY brin_insert_optimization_idx;
 DROP TABLE brin_insert_optimization;