Redesign API presented by nodeAgg.c for ordered-set and similar aggregates.

The previous design exposed the input and output ExprContexts of the
Agg plan node, but work on grouping sets has suggested that we'll regret
doing that.  Instead provide more narrowly-defined APIs that can be
implemented in multiple ways, namely a way to get a short-term memory
context and a way to register an aggregate shutdown callback.

Back-patch to 9.4 where the bad APIs were introduced, since we don't
want third-party code using these APIs and then having to change in 9.5.

Andrew Gierth

diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 09ff03543df53b684570c07698c9e1f79538c3f6..510d1c50e9c5559bfa70fb4fbdf15a006029b5bc 100644 (file)
--- 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 for aggregates
  *
- * 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 for an aggregate
  *
  * This is useful for aggs to register shutdown callbacks, which will ensure
- * that non-memory resources are freed.
+ * that non-memory resources are freed.  The callback will occur just before
+ * the associated aggcontext (as returned by AggCheckCallContext) is reset,
+ * either between groups or as a result of rescanning the query.  The callback
+ * will NOT be called on error paths.  The typical use-case is for freeing of
+ * tuplestores or tuplesorts maintained in aggcontext, or pins held by slots
+ * created by the agg functions.  (The callback 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 efb0411c2286b644743b90ab307de05f1a0aa900..d116a630355ccb06e6b9a6bdd54b06d95c38b6b3 100644 (file)
--- 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 267403c410fd66669cef98e33cf26de12f677ea9..a901770948a07d97e419f619379c49b2625d8f40 100644 (file)
--- a/src/include/fmgr.h
+++ b/src/include/fmgr.h
@@ -23,7 +23,7 @@ typedef struct Node *fmNodePtr;
 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 +656,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