Re: Out-of-Memory with ROWTYPE assignment in PL/pgSQL FOR loop - Mailing list pgsql-bugs

"Bill Rugolsky Jr." <brugolsky@telemetry-investments.com> writes:
> The PL/pgSQL FOR loop in the function consume_memory() defined below
> will consume VM on each iteration until the process hits its ulimit.
> The problem occurs with variables of ROWTYPE; there is no unbounded
> allocation when using simple types such as integer or varchar.

Yeah, looks like I introduced a memory leak with the 8.0 changes for
better support of rowtype variables :-(.  Here's the patch.

            regards, tom lane


Index: pl_exec.c
===================================================================
RCS file: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v
retrieving revision 1.127.4.2
diff -c -r1.127.4.2 pl_exec.c
*** pl_exec.c    20 Jun 2005 20:44:50 -0000    1.127.4.2
--- pl_exec.c    20 Jun 2005 22:46:14 -0000
***************
*** 2003,2013 ****
      estate->eval_tuptable = NULL;
      estate->eval_processed = 0;
      estate->eval_lastoid = InvalidOid;
-     estate->eval_econtext = NULL;

      estate->err_func = func;
      estate->err_stmt = NULL;
      estate->err_text = NULL;
  }

  /* ----------
--- 2003,2032 ----
      estate->eval_tuptable = NULL;
      estate->eval_processed = 0;
      estate->eval_lastoid = InvalidOid;

      estate->err_func = func;
      estate->err_stmt = NULL;
      estate->err_text = NULL;
+
+     /*
+      * Create an EState for evaluation of simple expressions, if there's
+      * not one already in the current transaction.    The EState is made a
+      * child of TopTransactionContext so it will have the right lifespan.
+      */
+     if (simple_eval_estate == NULL)
+     {
+         MemoryContext oldcontext;
+
+         oldcontext = MemoryContextSwitchTo(TopTransactionContext);
+         simple_eval_estate = CreateExecutorState();
+         MemoryContextSwitchTo(oldcontext);
+     }
+
+     /*
+      * Create an expression context for simple expressions.
+      * This must be a child of simple_eval_estate.
+      */
+     estate->eval_econtext = CreateExprContext(simple_eval_estate);
  }

  /* ----------
***************
*** 3238,3243 ****
--- 3257,3264 ----
                  Datum *value,
                  bool *isnull)
  {
+     MemoryContext oldcontext;
+
      switch (datum->dtype)
      {
          case PLPGSQL_DTYPE_VAR:
***************
*** 3264,3272 ****
--- 3285,3295 ----
                      elog(ERROR, "row variable has no tupdesc");
                  /* Make sure we have a valid type/typmod setting */
                  BlessTupleDesc(row->rowtupdesc);
+                 oldcontext = MemoryContextSwitchTo(estate->eval_econtext->ecxt_per_tuple_memory);
                  tup = make_tuple_from_row(estate, row, row->rowtupdesc);
                  if (tup == NULL)    /* should not happen */
                      elog(ERROR, "row not compatible with its own tupdesc");
+                 MemoryContextSwitchTo(oldcontext);
                  *typeid = row->rowtupdesc->tdtypeid;
                  *value = HeapTupleGetDatum(tup);
                  *isnull = false;
***************
*** 3299,3308 ****
--- 3322,3333 ----
                   * fields. Copy the tuple body and insert the right
                   * values.
                   */
+                 oldcontext = MemoryContextSwitchTo(estate->eval_econtext->ecxt_per_tuple_memory);
                  heap_copytuple_with_tuple(rec->tup, &worktup);
                  HeapTupleHeaderSetDatumLength(worktup.t_data, worktup.t_len);
                  HeapTupleHeaderSetTypeId(worktup.t_data, rec->tupdesc->tdtypeid);
                  HeapTupleHeaderSetTypMod(worktup.t_data, rec->tupdesc->tdtypmod);
+                 MemoryContextSwitchTo(oldcontext);
                  *typeid = rec->tupdesc->tdtypeid;
                  *value = HeapTupleGetDatum(&worktup);
                  *isnull = false;
***************
*** 3579,3585 ****
                        Oid *rettype)
  {
      Datum        retval;
!     ExprContext * volatile econtext;
      ParamListInfo paramLI;
      int            i;
      Snapshot    saveActiveSnapshot;
--- 3604,3610 ----
                        Oid *rettype)
  {
      Datum        retval;
!     ExprContext *econtext = estate->eval_econtext;
      ParamListInfo paramLI;
      int            i;
      Snapshot    saveActiveSnapshot;
***************
*** 3590,3609 ****
      *rettype = expr->expr_simple_type;

      /*
-      * Create an EState for evaluation of simple expressions, if there's
-      * not one already in the current transaction.    The EState is made a
-      * child of TopTransactionContext so it will have the right lifespan.
-      */
-     if (simple_eval_estate == NULL)
-     {
-         MemoryContext oldcontext;
-
-         oldcontext = MemoryContextSwitchTo(TopTransactionContext);
-         simple_eval_estate = CreateExecutorState();
-         MemoryContextSwitchTo(oldcontext);
-     }
-
-     /*
       * Prepare the expression for execution, if it's not been done already
       * in the current transaction.
       */
--- 3615,3620 ----
***************
*** 3617,3634 ****
      }

      /*
-      * Create an expression context for simple expressions, if there's not
-      * one already in the current function call.  This must be a child of
-      * simple_eval_estate.
-      */
-     econtext = estate->eval_econtext;
-     if (econtext == NULL)
-     {
-         econtext = CreateExprContext(simple_eval_estate);
-         estate->eval_econtext = econtext;
-     }
-
-     /*
       * Param list can live in econtext's temporary memory context.
       *
       * XXX think about avoiding repeated palloc's for param lists? Beware
--- 3628,3633 ----