Commit 467963c3 authored by Tom Lane's avatar Tom Lane

Prevent query-lifespan memory leakage of SP-GiST traversal values.

The original coding of the SP-GiST scan traversalValue feature (commit
ccd6eb49) arranged for traversal values to be stored in the query's main
executor context.  That's fine if there's only one index scan per query,
but if there are many, we have a memory leak as successive scans create
new traversal values.  Fix it by creating a separate memory context for
traversal values, which we can reset during spgrescan().  Back-patch
to 9.6 where this code was introduced.

In principle, adding the traversalCxt field to SpGistScanOpaqueData
creates an ABI break in the back branches.  But I (tgl) have little
sympathy for extensions including spgist_private.h, so I'm not very
worried about that.  Alternatively we could stick the new field at the
end of the struct in back branches, but that has its own downsides.

Anton Dignös, reviewed by Alexander Kuzmenkov

Discussion: https://postgr.es/m/CALNdv1jb6y2Te-m8xHLxLX12RsBmZJ1f4hESX7J0HjgyOhA9eA@mail.gmail.com
parent 13c7c65e
...@@ -194,6 +194,9 @@ spgbeginscan(Relation rel, int keysz, int orderbysz) ...@@ -194,6 +194,9 @@ spgbeginscan(Relation rel, int keysz, int orderbysz)
so->tempCxt = AllocSetContextCreate(CurrentMemoryContext, so->tempCxt = AllocSetContextCreate(CurrentMemoryContext,
"SP-GiST search temporary context", "SP-GiST search temporary context",
ALLOCSET_DEFAULT_SIZES); ALLOCSET_DEFAULT_SIZES);
so->traversalCxt = AllocSetContextCreate(CurrentMemoryContext,
"SP-GiST traversal-value context",
ALLOCSET_DEFAULT_SIZES);
/* Set up indexTupDesc and xs_hitupdesc in case it's an index-only scan */ /* Set up indexTupDesc and xs_hitupdesc in case it's an index-only scan */
so->indexTupDesc = scan->xs_hitupdesc = RelationGetDescr(rel); so->indexTupDesc = scan->xs_hitupdesc = RelationGetDescr(rel);
...@@ -209,6 +212,9 @@ spgrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys, ...@@ -209,6 +212,9 @@ spgrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
{ {
SpGistScanOpaque so = (SpGistScanOpaque) scan->opaque; SpGistScanOpaque so = (SpGistScanOpaque) scan->opaque;
/* clear traversal context before proceeding to the next scan */
MemoryContextReset(so->traversalCxt);
/* copy scankeys into local storage */ /* copy scankeys into local storage */
if (scankey && scan->numberOfKeys > 0) if (scankey && scan->numberOfKeys > 0)
{ {
...@@ -229,6 +235,7 @@ spgendscan(IndexScanDesc scan) ...@@ -229,6 +235,7 @@ spgendscan(IndexScanDesc scan)
SpGistScanOpaque so = (SpGistScanOpaque) scan->opaque; SpGistScanOpaque so = (SpGistScanOpaque) scan->opaque;
MemoryContextDelete(so->tempCxt); MemoryContextDelete(so->tempCxt);
MemoryContextDelete(so->traversalCxt);
} }
/* /*
...@@ -463,7 +470,7 @@ redirect: ...@@ -463,7 +470,7 @@ redirect:
in.scankeys = so->keyData; in.scankeys = so->keyData;
in.nkeys = so->numberOfKeys; in.nkeys = so->numberOfKeys;
in.reconstructedValue = stackEntry->reconstructedValue; in.reconstructedValue = stackEntry->reconstructedValue;
in.traversalMemoryContext = oldCtx; in.traversalMemoryContext = so->traversalCxt;
in.traversalValue = stackEntry->traversalValue; in.traversalValue = stackEntry->traversalValue;
in.level = stackEntry->level; in.level = stackEntry->level;
in.returnData = so->want_itup; in.returnData = so->want_itup;
......
...@@ -137,6 +137,7 @@ typedef struct SpGistScanOpaqueData ...@@ -137,6 +137,7 @@ typedef struct SpGistScanOpaqueData
{ {
SpGistState state; /* see above */ SpGistState state; /* see above */
MemoryContext tempCxt; /* short-lived memory context */ MemoryContext tempCxt; /* short-lived memory context */
MemoryContext traversalCxt; /* memory context for traversalValues */
/* Control flags showing whether to search nulls and/or non-nulls */ /* Control flags showing whether to search nulls and/or non-nulls */
bool searchNulls; /* scan matches (all) null entries */ bool searchNulls; /* scan matches (all) null entries */
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment