v1-0003-Assert-that-ExecOpenIndices-and-ExecCloseIndices-.patch

From 484115dc9598cc496040ef0058239f8f61fea457 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Tue, 18 Feb 2025 12:47:57 -0500
Subject: [PATCH v1 3/4] Assert that ExecOpenIndices and ExecCloseIndices are
 not repeated.

These functions should be called at most once per ResultRelInfo;
it's wasteful to do otherwise, and certainly the pattern of
opening twice and then closing twice is a bad idea.  Moreover,
index_insert_cleanup functions might not be prepared to be
called twice, as the just-hardened code in BRIN demonstrates.

Sadly, logical replication's apply_handle_tuple_routing() does
exactly that, meaning that either hunk of this patch is
sufficient to make it crash.

While other patches in this series are back-patch candidates,
this one should only be applied to HEAD, as perhaps there
are extensions doing the now-forbidden coding pattern.
We shouldn't break them in minor releases.

Bug: #18815
Reported-by: Sergey Belyashov <sergey.belyashov@gmail.com>
Discussion: https://postgr.es/m/18815-2a0407cc7f40b327@postgresql.org
---
 src/backend/executor/execIndexing.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c
index 7c87f012c3..742f3f8c08 100644
--- a/src/backend/executor/execIndexing.c
+++ b/src/backend/executor/execIndexing.c
@@ -181,6 +181,9 @@ ExecOpenIndices(ResultRelInfo *resultRelInfo, bool speculative)
 	if (len == 0)
 		return;
 
+	/* This Assert will fail if ExecOpenIndices is called twice */
+	Assert(resultRelInfo->ri_IndexRelationDescs == NULL);
+
 	/*
 	 * allocate space for result arrays
 	 */
@@ -246,19 +249,23 @@ ExecCloseIndices(ResultRelInfo *resultRelInfo)
 
 	for (i = 0; i < numIndices; i++)
 	{
-		if (indexDescs[i] == NULL)
-			continue;			/* shouldn't happen? */
+		/* This Assert will fail if ExecCloseIndices is called twice */
+		Assert(indexDescs[i] != NULL);
 
 		/* Give the index a chance to do some post-insert cleanup */
 		index_insert_cleanup(indexDescs[i], indexInfos[i]);
 
 		/* Drop lock acquired by ExecOpenIndices */
 		index_close(indexDescs[i], RowExclusiveLock);
+
+		/* Mark the index as closed */
+		indexDescs[i] = NULL;
 	}
 
 	/*
-	 * XXX should free indexInfo array here too?  Currently we assume that
-	 * such stuff will be cleaned up automatically in FreeExecutorState.
+	 * We don't attempt to free the IndexInfo data structures or the arrays,
+	 * instead assuming that such stuff will be cleaned up automatically in
+	 * FreeExecutorState.
 	 */
 }
 
-- 
2.43.5