exec_eval_datum leaks memory when dealing with ROW or REC values.
authorTom Lane
Mon, 20 Jun 2005 22:51:29 +0000 (22:51 +0000)
committerTom Lane
Mon, 20 Jun 2005 22:51:29 +0000 (22:51 +0000)
It never leaked memory before PG 8.0, so none of the callers are
expecting this.  Cleanest fix seems to be to make it allocate the needed
memory in estate->eval_econtext, where it will be cleaned up by
the next exec_eval_cleanup.  Per report from Bill Rugolsky.

src/pl/plpgsql/src/pl_exec.c

index d09c9fc234270af3eaf652b04970d538eb15e666..04403fa5640b01d4d98aa52d0b4a14c059094b6d 100644 (file)
@@ -3,7 +3,7 @@
  *           procedural language
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.145 2005/06/20 20:44:44 tgl Exp $
+ *   $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.146 2005/06/20 22:51:29 tgl Exp $
  *
  *   This software is copyrighted by Jan Wieck - Hamburg.
  *
@@ -2012,11 +2012,30 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
    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;
+
+   /*
+    * 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);
 }
 
 /* ----------
@@ -3264,6 +3283,8 @@ exec_eval_datum(PLpgSQL_execstate *estate,
                Datum *value,
                bool *isnull)
 {
+   MemoryContext oldcontext;
+
    switch (datum->dtype)
    {
        case PLPGSQL_DTYPE_VAR:
@@ -3290,9 +3311,11 @@ exec_eval_datum(PLpgSQL_execstate *estate,
                    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;
@@ -3325,10 +3348,12 @@ exec_eval_datum(PLpgSQL_execstate *estate,
                 * 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;
@@ -3605,7 +3630,7 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
                      Oid *rettype)
 {
    Datum       retval;
-   ExprContext * volatile econtext;
+   ExprContext *econtext = estate->eval_econtext;
    ParamListInfo paramLI;
    int         i;
    Snapshot    saveActiveSnapshot;
@@ -3615,20 +3640,6 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
     */
    *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.
@@ -3642,18 +3653,6 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
        active_simple_exprs = expr;
    }
 
-   /*
-    * 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.
     *