From 484115dc9598cc496040ef0058239f8f61fea457 Mon Sep 17 00:00:00 2001 From: Tom Lane 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 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