Don't cache per-group context across the whole query in orderedsetaggs.c.

Although nodeAgg.c currently uses the same per-group memory context for
all groups of a query, that might change in future.  Avoid assuming it.
This costs us an extra AggCheckCallContext() call per group, but that's
pretty cheap and is probably good from a safety standpoint anyway.

Back-patch to 9.4 in case any third-party code copies this logic.

Andrew Gierth
This commit is contained in:
Tom Lane 2014-07-03 18:47:09 -04:00
parent 6f5034eda0
commit ecd6579744
1 changed files with 15 additions and 13 deletions

View File

@ -47,8 +47,6 @@ typedef struct OSAPerQueryState
Aggref *aggref; Aggref *aggref;
/* Memory context containing this struct and other per-query data: */ /* Memory context containing this struct and other per-query data: */
MemoryContext qcontext; MemoryContext qcontext;
/* Memory context containing per-group data: */
MemoryContext gcontext;
/* These fields are used only when accumulating tuples: */ /* These fields are used only when accumulating tuples: */
@ -86,6 +84,8 @@ typedef struct OSAPerGroupState
{ {
/* Link to the per-query state for this aggregate: */ /* Link to the per-query state for this aggregate: */
OSAPerQueryState *qstate; OSAPerQueryState *qstate;
/* Memory context containing per-group data: */
MemoryContext gcontext;
/* Sort object we're accumulating data in: */ /* Sort object we're accumulating data in: */
Tuplesortstate *sortstate; Tuplesortstate *sortstate;
/* Number of normal rows inserted into sortstate: */ /* Number of normal rows inserted into sortstate: */
@ -103,8 +103,17 @@ ordered_set_startup(FunctionCallInfo fcinfo, bool use_tuples)
{ {
OSAPerGroupState *osastate; OSAPerGroupState *osastate;
OSAPerQueryState *qstate; OSAPerQueryState *qstate;
MemoryContext gcontext;
MemoryContext oldcontext; MemoryContext oldcontext;
/*
* Check we're called as aggregate (and not a window function), and get
* the Agg node's group-lifespan context (which might change from group to
* group, so we shouldn't cache it in the per-query state).
*/
if (AggCheckCallContext(fcinfo, &gcontext) != AGG_CONTEXT_AGGREGATE)
elog(ERROR, "ordered-set aggregate called in non-aggregate context");
/* /*
* We keep a link to the per-query state in fn_extra; if it's not there, * We keep a link to the per-query state in fn_extra; if it's not there,
* create it, and do the per-query setup we need. * create it, and do the per-query setup we need.
@ -114,17 +123,10 @@ ordered_set_startup(FunctionCallInfo fcinfo, bool use_tuples)
{ {
Aggref *aggref; Aggref *aggref;
MemoryContext qcontext; MemoryContext qcontext;
MemoryContext gcontext;
List *sortlist; List *sortlist;
int numSortCols; int numSortCols;
/* /* Get the Aggref so we can examine aggregate's arguments */
* Check we're called as aggregate (and not a window function), and
* get the Agg node's group-lifespan context
*/
if (AggCheckCallContext(fcinfo, &gcontext) != AGG_CONTEXT_AGGREGATE)
elog(ERROR, "ordered-set aggregate called in non-aggregate context");
/* Need the Aggref as well */
aggref = AggGetAggref(fcinfo); aggref = AggGetAggref(fcinfo);
if (!aggref) if (!aggref)
elog(ERROR, "ordered-set aggregate called in non-aggregate context"); elog(ERROR, "ordered-set aggregate called in non-aggregate context");
@ -142,7 +144,6 @@ ordered_set_startup(FunctionCallInfo fcinfo, bool use_tuples)
qstate = (OSAPerQueryState *) palloc0(sizeof(OSAPerQueryState)); qstate = (OSAPerQueryState *) palloc0(sizeof(OSAPerQueryState));
qstate->aggref = aggref; qstate->aggref = aggref;
qstate->qcontext = qcontext; qstate->qcontext = qcontext;
qstate->gcontext = gcontext;
/* Extract the sort information */ /* Extract the sort information */
sortlist = aggref->aggorder; sortlist = aggref->aggorder;
@ -259,10 +260,11 @@ ordered_set_startup(FunctionCallInfo fcinfo, bool use_tuples)
} }
/* Now build the stuff we need in group-lifespan context */ /* Now build the stuff we need in group-lifespan context */
oldcontext = MemoryContextSwitchTo(qstate->gcontext); oldcontext = MemoryContextSwitchTo(gcontext);
osastate = (OSAPerGroupState *) palloc(sizeof(OSAPerGroupState)); osastate = (OSAPerGroupState *) palloc(sizeof(OSAPerGroupState));
osastate->qstate = qstate; osastate->qstate = qstate;
osastate->gcontext = gcontext;
/* Initialize tuplesort object */ /* Initialize tuplesort object */
if (use_tuples) if (use_tuples)
@ -282,7 +284,7 @@ ordered_set_startup(FunctionCallInfo fcinfo, bool use_tuples)
osastate->number_of_rows = 0; osastate->number_of_rows = 0;
/* Now register a shutdown callback to clean things up */ /* Now register a shutdown callback to clean things up at end of group */
AggRegisterCallback(fcinfo, AggRegisterCallback(fcinfo,
ordered_set_shutdown, ordered_set_shutdown,
PointerGetDatum(osastate)); PointerGetDatum(osastate));