Avoid leaking memory while evaluating arguments for a table function.

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.

diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c
index ab05ec226eab44ffd372fb1e0950bb59aff49619..c6f31a0ad0b672eef99575340cd8d7a69afe0eec 100644 (file)
--- a/src/backend/executor/execQual.c
+++ b/src/backend/executor/execQual.c
@@ -2002,6 +2002,7 @@ ExecMakeFunctionResultNoSets(FuncExprState *fcache,
 Tuplestorestate *
 ExecMakeTableFunctionResult(ExprState *funcexpr,
                                                        ExprContext *econtext,
+                                                       MemoryContext argContext,
                                                        TupleDesc expectedDesc,
                                                        bool randomAccess)
 {
@@ -2083,12 +2084,18 @@ 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);
                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,

diff --git a/src/backend/executor/nodeFunctionscan.c b/src/backend/executor/nodeFunctionscan.c
index e09c16177c3b2a65000bb69d440651223a89ec52..283f547b426f2757e6c34b66ef94414c02864ee3 100644 (file)
--- a/src/backend/executor/nodeFunctionscan.c
+++ b/src/backend/executor/nodeFunctionscan.c
@@ -25,6 +25,7 @@
 #include "executor/nodeFunctionscan.h"
 #include "funcapi.h"
 #include "nodes/nodeFuncs.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);
        }
@@ -227,6 +229,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;
 }
 

diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index f63869229301afb9334acd5341f565ff4b4e8922..4043b8f4483816d78d7a2da41150b4616ef86586 100644 (file)
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -229,6 +229,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,

diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 1344fc4adb65b44fc339e878ca9d4083d445be3a..c443112d806ad2b94b9ab9f3101a7ff815c433b2 100644 (file)
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1392,6 +1392,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
@@ -1401,6 +1402,7 @@ typedef struct FunctionScanState
        TupleDesc       tupdesc;
        Tuplestorestate *tuplestorestate;
        ExprState  *funcexpr;
+       MemoryContext argcontext;
 } FunctionScanState;
 
 /* ----------------