database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-04-24 10:47:04 +03:00
Code

Assert that ExecOpenIndices and ExecCloseIndices are not repeated.

Browse Source 
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,
aminsertcleanup functions might not be prepared to be called twice,
as the just-hardened code in BRIN demonstrates.

This amounts to an API change, since such coding patterns were
safe even if wasteful before v17.  Hence, apply to HEAD only.
(Extension code violating this new rule faces some risk in v17,
but we just fixed brininsertcleanup and there are probably few
other aminsertcleanup functions as yet.  So the odds of breaking
usable code seem higher than the odds of doing something useful
with a back-patch.)

Bug: #18815
Reported-by: Sergey Belyashov <sergey.belyashov@gmail.com>
Discussion: https://postgr.es/m/18815-2a0407cc7f40b327@postgresql.org
This commit is contained in:
Tom Lane 2025-02-19 16:45:12 -05:00
parent 9ff68679b5
commit e596e077bb
1 changed files with 11 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/backend/executor/execIndexing.c
View File

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