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.

diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c
index 9d73142ee93897d82f77d21a37e79f5b17604bc2..3e2b8b5fedf05a68be4368d78a3d87ddc5cf679d 100644 (file)
--- 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 b3a2de1edd4d37d72f6b665c8c1d7cd552dbb4f0..ac3a92b1981a372a04dc1799b9c96689554ba543 100644 (file)
--- 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 c949c97edec779bfa7220cc81886e5092f110538..bda7c284b132d4e18d620890670014099679f9d9 100644 (file)
--- 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;