Re: [HACKERS] aggregation memory leak and fix - Mailing list pgsql-hackers

> --
> -- Query 1
> --
> select l_returnflag, l_linestatus, sum(l_quantity) as sum_qty,
> sum(l_extendedprice) as sum_base_price,
> sum(l_extendedprice*(1-l_discount)) as sum_disc_price,
> sum(l_extendedprice*(1-l_discount)*(1+l_tax)) as sum_charge,
> avg(l_quantity) as avg_qty, avg(l_extendedprice) as avg_price,
> avg(l_discount) as avg_disc, count(*) as count_order
> from lineitem
> where l_shipdate <= ('1998-12-01'::datetime - interval '90 day')::date
> group by l_returnflag, l_linestatus
> order by l_returnflag, l_linestatus;
>
>
> when I run this against 6.4.2, the postgres process grows to upwards of
> 1 GB of memory (at which point something overflows and it dumps core) -
> I watch it grow through 200 MB, 400 MB, 800 MB, dies somewhere near 1 GB
> of allocated memory).
>

Here is my first attempt at fixing the expression memory leak you
mentioned.  I have run it through the regression tests, and it seems to
be harmless there.

I am interested to see if it fixes the expression leak you saw.  I have
not committed this yet.  I want to look at it some more.

--
  Bruce Momjian                        |  http://www.op.net/~candle
  maillist@candle.pha.pa.us            |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
Index: execQual.c
===================================================================
RCS file: /usr/local/cvsroot/pgsql/src/backend/executor/execQual.c,v
retrieving revision 1.50
diff -c -r1.50 execQual.c
*** execQual.c    1999/03/20 02:07:31    1.50
--- execQual.c    1999/03/24 06:09:25
***************
*** 51,56 ****
--- 51,57 ----
  #include "utils/fcache2.h"
  #include "utils/mcxt.h"
  #include "utils/memutils.h"
+ #include "utils/portal.h"


  /*
***************
*** 65,70 ****
--- 66,74 ----
  bool        execConstByVal;
  int            execConstLen;

+ bool        allocatedQualContext;
+ bool        allocatedQualNesting;
+
  /* static functions decls */
  static Datum ExecEvalAggref(Aggref *aggref, ExprContext *econtext, bool *isNull);
  static Datum ExecEvalArrayRef(ArrayRef *arrayRef, ExprContext *econtext,
***************
*** 801,814 ****
      else
      {
          int            i;

          if (isDone)
              *isDone = true;
          for (i = 0; i < fcache->nargs; i++)
              if (fcache->nullVect[i] == true)
                  *isNull = true;

!         return (Datum) fmgr_c(&fcache->func, (FmgrValues *) argV, isNull);
      }
  }

--- 805,852 ----
      else
      {
          int            i;
+         Datum        d;
+         char        pname[64];
+         Portal         qual_portal;
+         MemoryContext oldcxt;

          if (isDone)
              *isDone = true;
          for (i = 0; i < fcache->nargs; i++)
              if (fcache->nullVect[i] == true)
                  *isNull = true;
+
+         /*
+          * Assign adt *.c memory in separate context to prevent
+          * unbounded memory growth in large queries that use functions.
+          * We clear this memory after the qual has been completed.
+          * bjm 1999/03/24
+          */
+         strcpy(pname, "<Qual manager>");
+         qual_portal = GetPortalByName(pname);
+         if (!PortalIsValid(qual_portal))
+         {
+             qual_portal = CreatePortal(pname);
+             Assert(PortalIsValid(qual_portal));

!             oldcxt = MemoryContextSwitchTo(
!                     (MemoryContext) PortalGetHeapMemory(qual_portal));
!             StartPortalAllocMode(DefaultAllocMode, 0);
!             MemoryContextSwitchTo(oldcxt);
!
!              allocatedQualContext = true;
!              allocatedQualNesting = 0;
!         }
!          allocatedQualNesting++;
!
!         oldcxt = MemoryContextSwitchTo(
!                 (MemoryContext) PortalGetHeapMemory(qual_portal));
!
!         d = (Datum) fmgr_c(&fcache->func, (FmgrValues *) argV, isNull);
!
!         MemoryContextSwitchTo(oldcxt);
!          allocatedQualNesting--;
!         return d;
      }
  }

***************
*** 1354,1359 ****
--- 1392,1399 ----
  {
      List       *clause;
      bool        result;
+     char        pname[64];
+     Portal         qual_portal;

      /*
       *    debugging stuff
***************
*** 1387,1396 ****
              break;
      }

      /*
       *    if result is true, then it means a clause failed so we
       *    return false.  if result is false then it means no clause
!      *    failed so we return true.
       */
      if (result == true)
          return false;
--- 1427,1459 ----
              break;
      }

+     if (allocatedQualContext && allocatedQualNesting == 0)
+     {
+         MemoryContext oldcxt;
+
+         strcpy(pname, "<Qual manager>");
+         qual_portal = GetPortalByName(pname);
+         /*
+          *    allocatedQualContext may have been improperly set from
+          *    from a previous run.
+          */
+         if (PortalIsValid(qual_portal))
+         {
+             oldcxt = MemoryContextSwitchTo(
+                     (MemoryContext) PortalGetHeapMemory(qual_portal));
+
+             EndPortalAllocMode();
+             StartPortalAllocMode(DefaultAllocMode, 0);
+
+             MemoryContextSwitchTo(oldcxt);
+         }
+         allocatedQualContext = false;
+     }
+
      /*
       *    if result is true, then it means a clause failed so we
       *    return false.  if result is false then it means no clause
!      *    failed so we return true.  ...Yikes, who wrote that?
       */
      if (result == true)
          return false;