Avoid leaking memory while evaluating arguments for a table function.
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 20 Jun 2014 02:14:00 +0000 (22:14 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 20 Jun 2014 02:14:00 +0000 (22:14 -0400)
ExecMakeTableFunctionResult evaluated the arguments for a function-in-FROM
in the query-lifespan memory context.  This is insignificant in simple
cases where the function relation is scanned only once; but if the function
is in a sub-SELECT or is on the inside of a nested loop, any memory
consumed during argument evaluation can add up quickly.  (The potential for
trouble here had been foreseen long ago, per existing comments; but we'd
not previously seen a complaint from the field about it.)  To fix, create
an additional temporary context just for this purpose.

Per an example from MauMau.  Back-patch to all active branches.

src/backend/executor/execQual.c
src/backend/executor/nodeFunctionscan.c
src/include/executor/executor.h
src/include/nodes/execnodes.h

index 97c7d175d323766cbd00407b89d95d317f0b9637..8410032cf50c61e835061ba92a8c72b759b6bc4d 100644 (file)
@@ -1935,6 +1935,7 @@ ExecMakeFunctionResultNoSets(FuncExprState *fcache,
 Tuplestorestate *
 ExecMakeTableFunctionResult(ExprState *funcexpr,
                            ExprContext *econtext,
+                           MemoryContext argContext,
                            TupleDesc expectedDesc,
                            bool randomAccess)
 {
@@ -2013,13 +2014,19 @@ ExecMakeTableFunctionResult(ExprState *funcexpr,
        /*
         * Evaluate the function's argument list.
         *
-        * Note: ideally, we'd do this in the per-tuple context, but then the
-        * argument values would disappear when we reset the context in the
-        * inner loop.  So do it in caller context.  Perhaps we should make a
-        * separate context just to hold the evaluated arguments?
+        * We can't do this in the per-tuple context: the argument values
+        * would disappear when we reset that context in the inner loop.  And
+        * the caller's CurrentMemoryContext is typically a query-lifespan
+        * context, so we don't want to leak memory there.  We require the
+        * caller to pass a separate memory context that can be used for this,
+        * and can be reset each time through to avoid bloat.
         */
+       MemoryContextReset(argContext);
+       oldcontext = MemoryContextSwitchTo(argContext);
        fcinfo.flinfo = &(fcache->func);
        argDone = ExecEvalFuncArgs(&fcinfo, fcache->args, econtext);
+       MemoryContextSwitchTo(oldcontext);
+
        /* We don't allow sets in the arguments of the table function */
        if (argDone != ExprSingleResult)
            ereport(ERROR,
index 8cb02b31edbb2897a573ba5a7c8b0cc70da60a4a..fe06cb26b417567dcf06d3e2810a2d4813c89fda 100644 (file)
@@ -25,6 +25,7 @@
 #include "executor/nodeFunctionscan.h"
 #include "funcapi.h"
 #include "utils/builtins.h"
+#include "utils/memutils.h"
 
 
 static TupleTableSlot *FunctionNext(FunctionScanState *node);
@@ -64,6 +65,7 @@ FunctionNext(FunctionScanState *node)
        node->tuplestorestate = tuplestorestate =
            ExecMakeTableFunctionResult(node->funcexpr,
                                        node->ss.ps.ps_ExprContext,
+                                       node->argcontext,
                                        node->tupdesc,
                                        node->eflags & EXEC_FLAG_BACKWARD);
    }
@@ -217,6 +219,19 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate, int eflags)
    ExecAssignResultTypeFromTL(&scanstate->ss.ps);
    ExecAssignScanProjectionInfo(&scanstate->ss);
 
+   /*
+    * Create a memory context that ExecMakeTableFunctionResult can use to
+    * evaluate function arguments in.  We can't use the per-tuple context for
+    * this because it gets reset too often; but we don't want to leak
+    * evaluation results into the query-lifespan context either.  We just
+    * need one context, because we evaluate each function separately.
+    */
+   scanstate->argcontext = AllocSetContextCreate(CurrentMemoryContext,
+                                                 "Table function arguments",
+                                                 ALLOCSET_DEFAULT_MINSIZE,
+                                                 ALLOCSET_DEFAULT_INITSIZE,
+                                                 ALLOCSET_DEFAULT_MAXSIZE);
+
    return scanstate;
 }
 
index 6097f7804f99cb942ba8bd4498654b30ae7003b0..27d74378e2215b0ebda1e04f354bf4f453fcdbd1 100644 (file)
@@ -188,6 +188,7 @@ extern Datum GetAttributeByName(HeapTupleHeader tuple, const char *attname,
                   bool *isNull);
 extern Tuplestorestate *ExecMakeTableFunctionResult(ExprState *funcexpr,
                            ExprContext *econtext,
+                           MemoryContext argContext,
                            TupleDesc expectedDesc,
                            bool randomAccess);
 extern Datum ExecEvalExprSwitchContext(ExprState *expression, ExprContext *econtext,
index 027158998e6d6924465059ea63d8a64c079caf5b..3e8f11d9d6b977408c3d69d20fa79686673c0f94 100644 (file)
@@ -1240,6 +1240,7 @@ typedef struct SubqueryScanState
  *     tupdesc             expected return tuple description
  *     tuplestorestate     private state of tuplestore.c
  *     funcexpr            state for function expression being evaluated
+ *     argcontext          memory context to evaluate function arguments in
  * ----------------
  */
 typedef struct FunctionScanState
@@ -1249,6 +1250,7 @@ typedef struct FunctionScanState
    TupleDesc   tupdesc;
    Tuplestorestate *tuplestorestate;
    ExprState  *funcexpr;
+   MemoryContext argcontext;
 } FunctionScanState;
 
 /* ----------------