diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2014-06-19 22:13:41 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2014-06-19 22:14:26 -0400 |
commit | 45b0f357235236dd3198f8abcca277adc0d7459a (patch) | |
tree | f8a91f92510a5a8a0e62ef4698a2c878bf85e7f5 /src | |
parent | 686f362bee126e50280bcd3b35807b02f18a8966 (diff) | |
download | postgresql-45b0f357235236dd3198f8abcca277adc0d7459a.tar.gz postgresql-45b0f357235236dd3198f8abcca277adc0d7459a.zip |
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.
Diffstat (limited to 'src')
-rw-r--r-- | src/backend/executor/execQual.c | 15 | ||||
-rw-r--r-- | src/backend/executor/nodeFunctionscan.c | 16 | ||||
-rw-r--r-- | src/include/executor/executor.h | 1 | ||||
-rw-r--r-- | src/include/nodes/execnodes.h | 2 |
4 files changed, 30 insertions, 4 deletions
diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c index f162e92fc71..bc79e3aa00c 100644 --- 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 da5d8c114db..945a414e96f 100644 --- a/src/backend/executor/nodeFunctionscan.c +++ b/src/backend/executor/nodeFunctionscan.c @@ -28,6 +28,7 @@ #include "nodes/nodeFuncs.h" #include "parser/parsetree.h" #include "utils/builtins.h" +#include "utils/memutils.h" /* @@ -94,6 +95,7 @@ FunctionNext(FunctionScanState *node) node->funcstates[0].tstore = tstore = ExecMakeTableFunctionResult(node->funcstates[0].funcexpr, node->ss.ps.ps_ExprContext, + node->argcontext, node->funcstates[0].tupdesc, node->eflags & EXEC_FLAG_BACKWARD); @@ -152,6 +154,7 @@ FunctionNext(FunctionScanState *node) fs->tstore = ExecMakeTableFunctionResult(fs->funcexpr, node->ss.ps.ps_ExprContext, + node->argcontext, fs->tupdesc, node->eflags & EXEC_FLAG_BACKWARD); @@ -515,6 +518,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 5e4a15ca747..239aff32088 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -231,6 +231,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 1f7c6d13144..b271f213f59 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1407,6 +1407,7 @@ typedef struct SubqueryScanState * nfuncs number of functions being executed * funcstates per-function execution states (private in * nodeFunctionscan.c) + * argcontext memory context to evaluate function arguments in * ---------------- */ struct FunctionScanPerFuncState; @@ -1421,6 +1422,7 @@ typedef struct FunctionScanState int nfuncs; struct FunctionScanPerFuncState *funcstates; /* array of length * nfuncs */ + MemoryContext argcontext; } FunctionScanState; /* ---------------- |