From 45b0f357235236dd3198f8abcca277adc0d7459a Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 19 Jun 2014 22:13:41 -0400 Subject: [PATCH] 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. --- src/backend/executor/execQual.c | 15 +++++++++++---- src/backend/executor/nodeFunctionscan.c | 16 ++++++++++++++++ src/include/executor/executor.h | 1 + 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 f162e92fc7..bc79e3aa00 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 da5d8c114d..945a414e96 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 5e4a15ca74..239aff3208 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 1f7c6d1314..b271f213f5 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; /* ---------------- -- 2.40.0