Re: Hash join in SELECT target list expression keeps consuming memory

Amit Khandekar <amitdkhan.pg@gmail.com> writes:
> If the SELECT target list expression is a join subquery, and if the
> subquery does a hash join, then the query keeps on consuming more and
> more memory. Below is such a query :

Thanks for the report!

I dug into this with valgrind, and found that the problem is that
ExecHashTableCreate allocates some memory that isn't freed by
ExecHashTableDestroy, specifically the per-hash-key function
information.  This is just dumb.  We can keep that stuff in the
hashtable's hashCxt instead, where it will get freed at the right time.
The attached patch seems to fix it just by reordering the code.

I'm surprised nobody's noticed this before; maybe the problem is
of relatively recent vintage?  Haven't checked the back branches yet.

            regards, tom lane

diff --git a/src/backend/executor/nodeHash.c b/src/backend/executor/nodeHash.c
index 06bb44b..4f069d1 100644
*** a/src/backend/executor/nodeHash.c
--- b/src/backend/executor/nodeHash.c
*************** ExecHashTableCreate(HashState *state, Li
*** 472,478 ****
       * Initialize the hash table control block.
       *
       * The hashtable control block is just palloc'd from the executor's
!      * per-query memory context.
       */
      hashtable = (HashJoinTable) palloc(sizeof(HashJoinTableData));
      hashtable->nbuckets = nbuckets;
--- 472,479 ----
       * Initialize the hash table control block.
       *
       * The hashtable control block is just palloc'd from the executor's
!      * per-query memory context.  Everything else should be kept inside the
!      * subsidiary hashCxt or batchCxt.
       */
      hashtable = (HashJoinTable) palloc(sizeof(HashJoinTableData));
      hashtable->nbuckets = nbuckets;
*************** ExecHashTableCreate(HashState *state, Li
*** 515,520 ****
--- 516,537 ----
  #endif

      /*
+      * Create temporary memory contexts in which to keep the hashtable working
+      * storage.  See notes in executor/hashjoin.h.
+      */
+     hashtable->hashCxt = AllocSetContextCreate(CurrentMemoryContext,
+                                                "HashTableContext",
+                                                ALLOCSET_DEFAULT_SIZES);
+
+     hashtable->batchCxt = AllocSetContextCreate(hashtable->hashCxt,
+                                                 "HashBatchContext",
+                                                 ALLOCSET_DEFAULT_SIZES);
+
+     /* Allocate data that will live for the life of the hashjoin */
+
+     oldcxt = MemoryContextSwitchTo(hashtable->hashCxt);
+
+     /*
       * Get info about the hash functions to be used for each hash key. Also
       * remember whether the join operators are strict.
       */
*************** ExecHashTableCreate(HashState *state, Li
*** 540,561 ****
          i++;
      }

-     /*
-      * Create temporary memory contexts in which to keep the hashtable working
-      * storage.  See notes in executor/hashjoin.h.
-      */
-     hashtable->hashCxt = AllocSetContextCreate(CurrentMemoryContext,
-                                                "HashTableContext",
-                                                ALLOCSET_DEFAULT_SIZES);
-
-     hashtable->batchCxt = AllocSetContextCreate(hashtable->hashCxt,
-                                                 "HashBatchContext",
-                                                 ALLOCSET_DEFAULT_SIZES);
-
-     /* Allocate data that will live for the life of the hashjoin */
-
-     oldcxt = MemoryContextSwitchTo(hashtable->hashCxt);
-
      if (nbatch > 1 && hashtable->parallel_state == NULL)
      {
          /*
--- 557,562 ----