>>>>> "Tom" == Tom Lane <t...@sss.pgh.pa.us> writes:
Tom> If we're going to do that, I think it needs to be in 9.4. Are Tom> you going to work up a patch? How's this? (applies and passes regression on 9.4 and master) -- Andrew (irc:RhodiumToad)
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 09ff035..0388735 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2199,44 +2199,56 @@ AggGetAggref(FunctionCallInfo fcinfo) } /* - * AggGetPerTupleEContext - fetch per-input-tuple ExprContext + * AggGetTempMemoryContext - fetch short-term memory context * - * This is useful in agg final functions; the econtext returned is the - * same per-tuple context that the transfn was called in (which can - * safely get reset during the final function). + * This is useful in agg final functions; the context returned is one that the + * final function can safely reset as desired. This isn't useful for transition + * functions, since the context returned MAY (we don't promise) be the same as + * the context those are called in. * * As above, this is currently not useful for aggs called as window functions. */ -ExprContext * -AggGetPerTupleEContext(FunctionCallInfo fcinfo) +MemoryContext +AggGetTempMemoryContext(FunctionCallInfo fcinfo) { if (fcinfo->context && IsA(fcinfo->context, AggState)) { AggState *aggstate = (AggState *) fcinfo->context; - return aggstate->tmpcontext; + return aggstate->tmpcontext->ecxt_per_tuple_memory; } return NULL; } /* - * AggGetPerAggEContext - fetch per-output-tuple ExprContext + * AggRegisterCallback - register a cleanup callback linked to aggcontext * * This is useful for aggs to register shutdown callbacks, which will ensure - * that non-memory resources are freed. + * that non-memory resources are freed. These callbacks will occur just before + * the associated aggcontext (as returned by AggCheckCallContext) is reset, + * either between groups or as a result of rescanning the query. They will NOT + * be called on error paths. The primary intent is to allow for freeing of + * tuplestores or tuplesorts maintained in aggcontext, or pins held by slots + * created by the agg functions. (They will not be called until after the + * result of the finalfn is no longer needed, so it's safe for the finalfn to + * return data that will be freed by the callback.) * * As above, this is currently not useful for aggs called as window functions. */ -ExprContext * -AggGetPerAggEContext(FunctionCallInfo fcinfo) +void +AggRegisterCallback(FunctionCallInfo fcinfo, + ExprContextCallbackFunction func, + Datum arg) { if (fcinfo->context && IsA(fcinfo->context, AggState)) { AggState *aggstate = (AggState *) fcinfo->context; - return aggstate->ss.ps.ps_ExprContext; + RegisterExprContextCallback(aggstate->ss.ps.ps_ExprContext, func, arg); + + return; } - return NULL; + elog(ERROR,"Aggregate function cannot register a callback in this context"); } diff --git a/src/backend/utils/adt/orderedsetaggs.c b/src/backend/utils/adt/orderedsetaggs.c index efb0411..d116a63 100644 --- a/src/backend/utils/adt/orderedsetaggs.c +++ b/src/backend/utils/adt/orderedsetaggs.c @@ -49,8 +49,6 @@ typedef struct OSAPerQueryState MemoryContext qcontext; /* Memory context containing per-group data: */ MemoryContext gcontext; - /* Agg plan node's output econtext: */ - ExprContext *peraggecontext; /* These fields are used only when accumulating tuples: */ @@ -117,7 +115,6 @@ ordered_set_startup(FunctionCallInfo fcinfo, bool use_tuples) Aggref *aggref; MemoryContext qcontext; MemoryContext gcontext; - ExprContext *peraggecontext; List *sortlist; int numSortCols; @@ -133,10 +130,6 @@ ordered_set_startup(FunctionCallInfo fcinfo, bool use_tuples) elog(ERROR, "ordered-set aggregate called in non-aggregate context"); if (!AGGKIND_IS_ORDERED_SET(aggref->aggkind)) elog(ERROR, "ordered-set aggregate support function called for non-ordered-set aggregate"); - /* Also get output exprcontext so we can register shutdown callback */ - peraggecontext = AggGetPerAggEContext(fcinfo); - if (!peraggecontext) - elog(ERROR, "ordered-set aggregate called in non-aggregate context"); /* * Prepare per-query structures in the fn_mcxt, which we assume is the @@ -150,7 +143,6 @@ ordered_set_startup(FunctionCallInfo fcinfo, bool use_tuples) qstate->aggref = aggref; qstate->qcontext = qcontext; qstate->gcontext = gcontext; - qstate->peraggecontext = peraggecontext; /* Extract the sort information */ sortlist = aggref->aggorder; @@ -291,9 +283,9 @@ ordered_set_startup(FunctionCallInfo fcinfo, bool use_tuples) osastate->number_of_rows = 0; /* Now register a shutdown callback to clean things up */ - RegisterExprContextCallback(qstate->peraggecontext, - ordered_set_shutdown, - PointerGetDatum(osastate)); + AggRegisterCallback(fcinfo, + ordered_set_shutdown, + PointerGetDatum(osastate)); MemoryContextSwitchTo(oldcontext); @@ -1310,7 +1302,7 @@ hypothetical_dense_rank_final(PG_FUNCTION_ARGS) sortColIdx = osastate->qstate->sortColIdx; /* Get short-term context we can use for execTuplesMatch */ - tmpcontext = AggGetPerTupleEContext(fcinfo)->ecxt_per_tuple_memory; + tmpcontext = AggGetTempMemoryContext(fcinfo); /* insert the hypothetical row into the sort */ slot = osastate->qstate->tupslot; diff --git a/src/include/fmgr.h b/src/include/fmgr.h index 267403c..9324dba 100644 --- a/src/include/fmgr.h +++ b/src/include/fmgr.h @@ -24,6 +24,7 @@ typedef struct Aggref *fmAggrefPtr; /* Likewise, avoid including execnodes.h here */ typedef struct ExprContext *fmExprContextPtr; +typedef void (*fmExprContextCallbackFunction) (Datum arg); /* Likewise, avoid including stringinfo.h here */ typedef struct StringInfoData *fmStringInfo; @@ -656,8 +657,10 @@ extern void **find_rendezvous_variable(const char *varName); extern int AggCheckCallContext(FunctionCallInfo fcinfo, MemoryContext *aggcontext); extern fmAggrefPtr AggGetAggref(FunctionCallInfo fcinfo); -extern fmExprContextPtr AggGetPerTupleEContext(FunctionCallInfo fcinfo); -extern fmExprContextPtr AggGetPerAggEContext(FunctionCallInfo fcinfo); +extern MemoryContext AggGetTempMemoryContext(FunctionCallInfo fcinfo); +extern void AggRegisterCallback(FunctionCallInfo fcinfo, + fmExprContextCallbackFunction func, + Datum arg); /* * We allow plugin modules to hook function entry/exit. This is intended
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers