From d88976cfa1302e8dccdcbfe55e9e29faee8c0cdf Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Wed, 4 Feb 2015 17:40:25 +0200
Subject: [PATCH] Use a separate memory context for GIN scan keys.

It was getting tedious to track and release all the different things that
form a scan key. We were leaking at least the queryCategories array, and
possibly more, on a rescan. That was visible if a GIN index was used in a
nested loop join. This also protects from leaks in extractQuery method.

No backpatching, given the lack of complaints from the field. Maybe later,
after this has received more field testing.
---
 src/backend/access/gin/ginget.c  |  5 +++-
 src/backend/access/gin/ginscan.c | 43 +++++++++++++++++---------------
 src/include/access/gin_private.h |  2 ++
 3 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c
index 9d73142ee9..3e2b8b5fed 100644
--- a/src/backend/access/gin/ginget.c
+++ b/src/backend/access/gin/ginget.c
@@ -497,7 +497,7 @@ startScanKey(GinState *ginstate, GinScanOpaque so, GinScanKey key)
 		}
 		/* i is now the last required entry. */
 
-		MemoryContextSwitchTo(oldCtx);
+		MemoryContextSwitchTo(so->keyCtx);
 
 		key->nrequired = i + 1;
 		key->nadditional = key->nentries - key->nrequired;
@@ -515,11 +515,14 @@ startScanKey(GinState *ginstate, GinScanOpaque so, GinScanKey key)
 	}
 	else
 	{
+		MemoryContextSwitchTo(so->keyCtx);
+
 		key->nrequired = 1;
 		key->nadditional = 0;
 		key->requiredEntries = palloc(1 * sizeof(GinScanEntry));
 		key->requiredEntries[0] = key->scanEntry[0];
 	}
+	MemoryContextSwitchTo(oldCtx);
 }
 
 static void
diff --git a/src/backend/access/gin/ginscan.c b/src/backend/access/gin/ginscan.c
index b3a2de1edd..ac3a92b198 100644
--- a/src/backend/access/gin/ginscan.c
+++ b/src/backend/access/gin/ginscan.c
@@ -44,6 +44,11 @@ ginbeginscan(PG_FUNCTION_ARGS)
 										ALLOCSET_DEFAULT_MINSIZE,
 										ALLOCSET_DEFAULT_INITSIZE,
 										ALLOCSET_DEFAULT_MAXSIZE);
+	so->keyCtx = AllocSetContextCreate(CurrentMemoryContext,
+									   "Gin scan key context",
+									   ALLOCSET_DEFAULT_MINSIZE,
+									   ALLOCSET_DEFAULT_INITSIZE,
+									   ALLOCSET_DEFAULT_MAXSIZE);
 	initGinState(&so->ginstate, scan->indexRelation);
 
 	scan->opaque = so;
@@ -227,6 +232,9 @@ ginFillScanKey(GinScanOpaque so, OffsetNumber attnum,
 	}
 }
 
+/*
+ * Release current scan keys, if any.
+ */
 void
 ginFreeScanKeys(GinScanOpaque so)
 {
@@ -235,38 +243,22 @@ ginFreeScanKeys(GinScanOpaque so)
 	if (so->keys == NULL)
 		return;
 
-	for (i = 0; i < so->nkeys; i++)
-	{
-		GinScanKey	key = so->keys + i;
-
-		pfree(key->scanEntry);
-		pfree(key->entryRes);
-		if (key->requiredEntries)
-			pfree(key->requiredEntries);
-		if (key->additionalEntries)
-			pfree(key->additionalEntries);
-	}
-
-	pfree(so->keys);
-	so->keys = NULL;
-	so->nkeys = 0;
-
 	for (i = 0; i < so->totalentries; i++)
 	{
 		GinScanEntry entry = so->entries[i];
 
 		if (entry->buffer != InvalidBuffer)
 			ReleaseBuffer(entry->buffer);
-		if (entry->list)
-			pfree(entry->list);
 		if (entry->matchIterator)
 			tbm_end_iterate(entry->matchIterator);
 		if (entry->matchBitmap)
 			tbm_free(entry->matchBitmap);
-		pfree(entry);
 	}
 
-	pfree(so->entries);
+	MemoryContextResetAndDeleteChildren(so->keyCtx);
+
+	so->keys = NULL;
+	so->nkeys = 0;
 	so->entries = NULL;
 	so->totalentries = 0;
 }
@@ -278,6 +270,14 @@ ginNewScanKey(IndexScanDesc scan)
 	GinScanOpaque so = (GinScanOpaque) scan->opaque;
 	int			i;
 	bool		hasNullQuery = false;
+	MemoryContext oldCtx;
+
+	/*
+	 * Allocate all the scan key information in the key context. (If
+	 * extractQuery leaks anything there, it won't be reset until the end of
+	 * scan or rescan, but that's OK.)
+	 */
+	oldCtx = MemoryContextSwitchTo(so->keyCtx);
 
 	/* if no scan keys provided, allocate extra EVERYTHING GinScanKey */
 	so->keys = (GinScanKey)
@@ -412,6 +412,8 @@ ginNewScanKey(IndexScanDesc scan)
 							 RelationGetRelationName(scan->indexRelation))));
 	}
 
+	MemoryContextSwitchTo(oldCtx);
+
 	pgstat_count_index_scan(scan->indexRelation);
 }
 
@@ -445,6 +447,7 @@ ginendscan(PG_FUNCTION_ARGS)
 	ginFreeScanKeys(so);
 
 	MemoryContextDelete(so->tempCtx);
+	MemoryContextDelete(so->keyCtx);
 
 	pfree(so);
 
diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h
index c949c97ede..bda7c284b1 100644
--- a/src/include/access/gin_private.h
+++ b/src/include/access/gin_private.h
@@ -888,6 +888,8 @@ typedef struct GinScanOpaqueData
 	uint32		totalentries;
 	uint32		allocentries;	/* allocated length of entries[] */
 
+	MemoryContext keyCtx;		/* used to hold key and entry data */
+
 	bool		isVoidRes;		/* true if query is unsatisfiable */
 } GinScanOpaqueData;