Create an official API function for C functions to use to check if they are

being called as aggregates, and to get the aggregate transition state memory
context if needed.  Use it instead of poking directly into AggState and
WindowAggState in places that shouldn't know so much.

We should have done this in 8.4, probably, but better late than never.

Revised version of a patch by Hitoshi Harada.

contrib/tsearch2/tsearch2.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/contrib/tsearch2/tsearch2.c,v 1.11 2010/01/02 16:57:32 momjian Exp $
+ *	  $PostgreSQL: pgsql/contrib/tsearch2/tsearch2.c,v 1.12 2010/02/08 20:39:51 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -422,15 +422,8 @@ tsa_rewrite_accum(PG_FUNCTION_ARGS)
 	MemoryContext aggcontext;
 	MemoryContext oldcontext;
 
-	if (fcinfo->context && IsA(fcinfo->context, AggState))
+	if (!AggCheckCallContext(fcinfo, &aggcontext))
-		aggcontext = ((AggState *) fcinfo->context)->aggcontext;
-	else if (fcinfo->context && IsA(fcinfo->context, WindowAggState))
-		aggcontext = ((WindowAggState *) fcinfo->context)->wincontext;
-	else
-	{
 		elog(ERROR, "tsa_rewrite_accum called in non-aggregate context");
-		aggcontext = NULL;		/* keep compiler quiet */
-	}
 
 	if (PG_ARGISNULL(0) || PG_GETARG_POINTER(0) == NULL)
 	{

doc/src/sgml/xaggr.sgml

@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/xaggr.sgml,v 1.38 2009/06/20 18:45:28 tgl Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/xaggr.sgml,v 1.39 2010/02/08 20:39:51 tgl Exp $ -->
 
  <sect1 id="xaggr">
   <title>User-Defined Aggregates</title>
@@ -166,14 +166,10 @@ SELECT attrelid::regclass, array_accum(atttypid::regtype)
 
   <para>
    A function written in C can detect that it is being called as an
-   aggregate transition or final function by seeing if it was passed
+   aggregate transition or final function by calling
-   an <structname>AggState</> or <structname>WindowAggState</> node
+   <function>AggCheckCallContext</>, for example:
-   as the function call <quote>context</>,
-   for example by:
 <programlisting>
-        if (fcinfo-&gt;context &amp;&amp;
+        if (AggCheckCallContext(fcinfo, NULL))
-            (IsA(fcinfo-&gt;context, AggState) ||
-             IsA(fcinfo-&gt;context, WindowAggState)))
 </programlisting>
    One reason for checking this is that when it is true for a transition
    function, the first input

src/backend/executor/nodeAgg.c

@@ -44,30 +44,34 @@
  *	  is used to run finalize functions and compute the output tuple;
  *	  this context can be reset once per output tuple.
  *
- *	  Beginning in PostgreSQL 8.1, the executor's AggState node is passed as
+ *	  The executor's AggState node is passed as the fmgr "context" value in
- *	  the fmgr "context" value in all transfunc and finalfunc calls.  It is
+ *	  all transfunc and finalfunc calls.  It is not recommended that the
- *	  not really intended that the transition functions will look into the
+ *	  transition functions look at the AggState node directly, but they can
- *	  AggState node, but they can use code like
+ *	  use AggCheckCallContext() to verify that they are being called by
- *			if (fcinfo->context && IsA(fcinfo->context, AggState))
+ *	  nodeAgg.c (and not as ordinary SQL functions).  The main reason a
- *	  to verify that they are being called by nodeAgg.c and not as ordinary
+ *	  transition function might want to know this is so that it can avoid
- *	  SQL functions.  The main reason a transition function might want to know
+ *	  palloc'ing a fixed-size pass-by-ref transition value on every call:
- *	  that is that it can avoid palloc'ing a fixed-size pass-by-ref transition
+ *	  it can instead just scribble on and return its left input.  Ordinarily
- *	  value on every call: it can instead just scribble on and return its left
+ *	  it is completely forbidden for functions to modify pass-by-ref inputs,
- *	  input.  Ordinarily it is completely forbidden for functions to modify
+ *	  but in the aggregate case we know the left input is either the initial
- *	  pass-by-ref inputs, but in the aggregate case we know the left input is
+ *	  transition value or a previous function result, and in either case its
- *	  either the initial transition value or a previous function result, and
+ *	  value need not be preserved.  See int8inc() for an example.  Notice that
- *	  in either case its value need not be preserved.  See int8inc() for an
+ *	  advance_transition_function() is coded to avoid a data copy step when
- *	  example.	Notice that advance_transition_function() is coded to avoid a
+ *	  the previous transition value pointer is returned.  Also, some
- *	  data copy step when the previous transition value pointer is returned.
+ *	  transition functions want to store working state in addition to the
- *	  Also, some transition functions make use of the aggcontext to store
+ *	  nominal transition value; they can use the memory context returned by
- *	  working state.
+ *	  AggCheckCallContext() to do that.
+ *
+ *	  Note: AggCheckCallContext() is available as of PostgreSQL 9.0.  The
+ *	  AggState is available as context in earlier releases (back to 8.1),
+ *	  but direct examination of the node is needed to use it before 9.0.
  *
  *
  * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/nodeAgg.c,v 1.171 2010/01/02 16:57:41 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/nodeAgg.c,v 1.172 2010/02/08 20:39:51 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1969,6 +1973,42 @@ ExecReScanAgg(AggState *node, ExprContext *exprCtxt)
 		ExecReScan(((PlanState *) node)->lefttree, exprCtxt);
 }
 
+/*
+ * AggCheckCallContext - test if a SQL function is being called as an aggregate
+ *
+ * The transition and/or final functions of an aggregate may want to verify
+ * that they are being called as aggregates, rather than as plain SQL
+ * functions.  They should use this function to do so.  The return value
+ * is nonzero if being called as an aggregate, or zero if not.  (Specific
+ * nonzero values are AGG_CONTEXT_AGGREGATE or AGG_CONTEXT_WINDOW, but more
+ * values could conceivably appear in future.)
+ *
+ * If aggcontext isn't NULL, the function also stores at *aggcontext the
+ * identity of the memory context that aggregate transition values are
+ * being stored in.
+ */
+int
+AggCheckCallContext(FunctionCallInfo fcinfo, MemoryContext *aggcontext)
+{
+	if (fcinfo->context && IsA(fcinfo->context, AggState))
+	{
+		if (aggcontext)
+			*aggcontext = ((AggState *) fcinfo->context)->aggcontext;
+		return AGG_CONTEXT_AGGREGATE;
+	}
+	if (fcinfo->context && IsA(fcinfo->context, WindowAggState))
+	{
+		if (aggcontext)
+			*aggcontext = ((WindowAggState *) fcinfo->context)->wincontext;
+		return AGG_CONTEXT_WINDOW;
+	}
+
+	/* this is just to prevent "uninitialized variable" warnings */
+	if (aggcontext)
+		*aggcontext = NULL;
+	return 0;
+}
+
 /*
  * aggregate_dummy - dummy execution routine for aggregate functions
  *

src/backend/utils/adt/array_userfuncs.c

@@ -6,13 +6,12 @@
  * Copyright (c) 2003-2010, PostgreSQL Global Development Group
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/array_userfuncs.c,v 1.33 2010/01/02 16:57:53 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/array_userfuncs.c,v 1.34 2010/02/08 20:39:51 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 #include "postgres.h"
 
-#include "nodes/execnodes.h"
 #include "utils/array.h"
 #include "utils/builtins.h"
 #include "utils/lsyscache.h"
@@ -484,15 +483,10 @@ array_agg_transfn(PG_FUNCTION_ARGS)
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 				 errmsg("could not determine input data type")));
 
-	if (fcinfo->context && IsA(fcinfo->context, AggState))
+	if (!AggCheckCallContext(fcinfo, &aggcontext))
-		aggcontext = ((AggState *) fcinfo->context)->aggcontext;
-	else if (fcinfo->context && IsA(fcinfo->context, WindowAggState))
-		aggcontext = ((WindowAggState *) fcinfo->context)->wincontext;
-	else
 	{
 		/* cannot be called directly because of internal-type argument */
 		elog(ERROR, "array_agg_transfn called in non-aggregate context");
-		aggcontext = NULL;		/* keep compiler quiet */
 	}
 
 	state = PG_ARGISNULL(0) ? NULL : (ArrayBuildState *) PG_GETARG_POINTER(0);
@@ -528,9 +522,7 @@ array_agg_finalfn(PG_FUNCTION_ARGS)
 		PG_RETURN_NULL();		/* returns null iff no input values */
 
 	/* cannot be called directly because of internal-type argument */
-	Assert(fcinfo->context &&
+	Assert(AggCheckCallContext(fcinfo, NULL));
-		   (IsA(fcinfo->context, AggState) ||
-			IsA(fcinfo->context, WindowAggState)));
 
 	state = (ArrayBuildState *) PG_GETARG_POINTER(0);
 

src/backend/utils/adt/float.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/float.c,v 1.164 2010/01/02 16:57:53 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/float.c,v 1.165 2010/02/08 20:39:51 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1765,13 +1765,11 @@ float8_accum(PG_FUNCTION_ARGS)
 	CHECKFLOATVAL(sumX2, isinf(transvalues[2]) || isinf(newval), true);
 
 	/*
-	 * If we're invoked by nodeAgg, we can cheat and modify our first
+	 * If we're invoked as an aggregate, we can cheat and modify our first
 	 * parameter in-place to reduce palloc overhead. Otherwise we construct a
 	 * new array with the updated transition data and return it.
 	 */
-	if (fcinfo->context &&
+	if (AggCheckCallContext(fcinfo, NULL))
-		(IsA(fcinfo->context, AggState) ||
-		 IsA(fcinfo->context, WindowAggState)))
 	{
 		transvalues[0] = N;
 		transvalues[1] = sumX;
@@ -1820,13 +1818,11 @@ float4_accum(PG_FUNCTION_ARGS)
 	CHECKFLOATVAL(sumX2, isinf(transvalues[2]) || isinf(newval), true);
 
 	/*
-	 * If we're invoked by nodeAgg, we can cheat and modify our first
+	 * If we're invoked as an aggregate, we can cheat and modify our first
 	 * parameter in-place to reduce palloc overhead. Otherwise we construct a
 	 * new array with the updated transition data and return it.
 	 */
-	if (fcinfo->context &&
+	if (AggCheckCallContext(fcinfo, NULL))
-		(IsA(fcinfo->context, AggState) ||
-		 IsA(fcinfo->context, WindowAggState)))
 	{
 		transvalues[0] = N;
 		transvalues[1] = sumX;
@@ -2039,13 +2035,11 @@ float8_regr_accum(PG_FUNCTION_ARGS)
 				  isinf(newvalY), true);
 
 	/*
-	 * If we're invoked by nodeAgg, we can cheat and modify our first
+	 * If we're invoked as an aggregate, we can cheat and modify our first
 	 * parameter in-place to reduce palloc overhead. Otherwise we construct a
 	 * new array with the updated transition data and return it.
 	 */
-	if (fcinfo->context &&
+	if (AggCheckCallContext(fcinfo, NULL))
-		(IsA(fcinfo->context, AggState) ||
-		 IsA(fcinfo->context, WindowAggState)))
 	{
 		transvalues[0] = N;
 		transvalues[1] = sumX;

src/backend/utils/adt/int8.c

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/int8.c,v 1.77 2010/01/07 04:53:34 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/int8.c,v 1.78 2010/02/08 20:39:51 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -19,7 +19,6 @@
 
 #include "funcapi.h"
 #include "libpq/pqformat.h"
-#include "nodes/nodes.h"
 #include "utils/int8.h"
 
 
@@ -654,15 +653,13 @@ int8inc(PG_FUNCTION_ARGS)
 {
 	/*
 	 * When int8 is pass-by-reference, we provide this special case to avoid
-	 * palloc overhead for COUNT(): when called from nodeAgg, we know that the
+	 * palloc overhead for COUNT(): when called as an aggregate, we know that
-	 * argument is modifiable local storage, so just update it in-place. (If
+	 * the argument is modifiable local storage, so just update it
-	 * int8 is pass-by-value, then of course this is useless as well as
+	 * in-place. (If int8 is pass-by-value, then of course this is useless as
-	 * incorrect, so just ifdef it out.)
+	 * well as incorrect, so just ifdef it out.)
 	 */
 #ifndef USE_FLOAT8_BYVAL		/* controls int8 too */
-	if (fcinfo->context &&
+	if (AggCheckCallContext(fcinfo, NULL))
-		(IsA(fcinfo->context, AggState) ||
-		 IsA(fcinfo->context, WindowAggState)))
 	{
 		int64	   *arg = (int64 *) PG_GETARG_POINTER(0);
 		int64		result;
@@ -680,7 +677,7 @@ int8inc(PG_FUNCTION_ARGS)
 	else
 #endif
 	{
-		/* Not called by nodeAgg, so just do it the dumb way */
+		/* Not called as an aggregate, so just do it the dumb way */
 		int64		arg = PG_GETARG_INT64(0);
 		int64		result;
 

src/backend/utils/adt/numeric.c

@@ -14,7 +14,7 @@
  * Copyright (c) 1998-2010, PostgreSQL Global Development Group
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/numeric.c,v 1.121 2010/01/07 04:53:34 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/numeric.c,v 1.122 2010/02/08 20:39:51 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2679,16 +2679,14 @@ int2_sum(PG_FUNCTION_ARGS)
 	}
 
 	/*
-	 * If we're invoked by nodeAgg, we can cheat and modify our first
+	 * If we're invoked as an aggregate, we can cheat and modify our first
 	 * parameter in-place to avoid palloc overhead. If not, we need to return
 	 * the new value of the transition variable. (If int8 is pass-by-value,
 	 * then of course this is useless as well as incorrect, so just ifdef it
 	 * out.)
 	 */
 #ifndef USE_FLOAT8_BYVAL		/* controls int8 too */
-	if (fcinfo->context &&
+	if (AggCheckCallContext(fcinfo, NULL))
-		(IsA(fcinfo->context, AggState) ||
-		 IsA(fcinfo->context, WindowAggState)))
 	{
 		int64	   *oldsum = (int64 *) PG_GETARG_POINTER(0);
 
@@ -2730,16 +2728,14 @@ int4_sum(PG_FUNCTION_ARGS)
 	}
 
 	/*
-	 * If we're invoked by nodeAgg, we can cheat and modify our first
+	 * If we're invoked as an aggregate, we can cheat and modify our first
 	 * parameter in-place to avoid palloc overhead. If not, we need to return
 	 * the new value of the transition variable. (If int8 is pass-by-value,
 	 * then of course this is useless as well as incorrect, so just ifdef it
 	 * out.)
 	 */
 #ifndef USE_FLOAT8_BYVAL		/* controls int8 too */
-	if (fcinfo->context &&
+	if (AggCheckCallContext(fcinfo, NULL))
-		(IsA(fcinfo->context, AggState) ||
-		 IsA(fcinfo->context, WindowAggState)))
 	{
 		int64	   *oldsum = (int64 *) PG_GETARG_POINTER(0);
 
@@ -2782,7 +2778,7 @@ int8_sum(PG_FUNCTION_ARGS)
 	}
 
 	/*
-	 * Note that we cannot special-case the nodeAgg case here, as we do for
+	 * Note that we cannot special-case the aggregate case here, as we do for
 	 * int2_sum and int4_sum: numeric is of variable size, so we cannot modify
 	 * our first parameter in-place.
 	 */
@@ -2820,13 +2816,11 @@ int2_avg_accum(PG_FUNCTION_ARGS)
 	Int8TransTypeData *transdata;
 
 	/*
-	 * If we're invoked by nodeAgg, we can cheat and modify our first
+	 * If we're invoked as an aggregate, we can cheat and modify our first
 	 * parameter in-place to reduce palloc overhead. Otherwise we need to make
 	 * a copy of it before scribbling on it.
 	 */
-	if (fcinfo->context &&
+	if (AggCheckCallContext(fcinfo, NULL))
-		(IsA(fcinfo->context, AggState) ||
-		 IsA(fcinfo->context, WindowAggState)))
 		transarray = PG_GETARG_ARRAYTYPE_P(0);
 	else
 		transarray = PG_GETARG_ARRAYTYPE_P_COPY(0);
@@ -2850,13 +2844,11 @@ int4_avg_accum(PG_FUNCTION_ARGS)
 	Int8TransTypeData *transdata;
 
 	/*
-	 * If we're invoked by nodeAgg, we can cheat and modify our first
+	 * If we're invoked as an aggregate, we can cheat and modify our first
 	 * parameter in-place to reduce palloc overhead. Otherwise we need to make
 	 * a copy of it before scribbling on it.
 	 */
-	if (fcinfo->context &&
+	if (AggCheckCallContext(fcinfo, NULL))
-		(IsA(fcinfo->context, AggState) ||
-		 IsA(fcinfo->context, WindowAggState)))
 		transarray = PG_GETARG_ARRAYTYPE_P(0);
 	else
 		transarray = PG_GETARG_ARRAYTYPE_P_COPY(0);

src/backend/utils/adt/varlena.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/varlena.c,v 1.175 2010/02/01 03:14:43 itagaki Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/varlena.c,v 1.176 2010/02/08 20:39:51 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -21,7 +21,6 @@
 #include "libpq/md5.h"
 #include "libpq/pqformat.h"
 #include "miscadmin.h"
-#include "nodes/execnodes.h"
 #include "parser/scansup.h"
 #include "regex/regex.h"
 #include "utils/builtins.h"
@@ -74,7 +73,7 @@ static bytea *bytea_substring(Datum str,
 				int L,
 				bool length_not_specified);
 static bytea *bytea_overlay(bytea *t1, bytea *t2, int sp, int sl);
-static StringInfo makeStringAggState(fmNodePtr context);
+static StringInfo makeStringAggState(FunctionCallInfo fcinfo);
 
 
 /*****************************************************************************
@@ -3327,25 +3326,25 @@ pg_column_size(PG_FUNCTION_ARGS)
  * actually used at all, and on subsequent calls the delimiter precedes
  * the associated value.
  */
+
+/* subroutine to initialize state */
 static StringInfo
-makeStringAggState(fmNodePtr context)
+makeStringAggState(FunctionCallInfo fcinfo)
 {
 	StringInfo		state;
 	MemoryContext	aggcontext;
 	MemoryContext	oldcontext;
 
-	if (context && IsA(context, AggState))
+	if (!AggCheckCallContext(fcinfo, &aggcontext))
-		aggcontext = ((AggState *) context)->aggcontext;
-	else if (context && IsA(context, WindowAggState))
-		aggcontext = ((WindowAggState *) context)->wincontext;
-	else
 	{
 		/* cannot be called directly because of internal-type argument */
 		elog(ERROR, "string_agg_transfn called in non-aggregate context");
-		aggcontext = NULL;		/* keep compiler quiet */
 	}
 
-	/* Create state in aggregate context */
+	/*
+	 * Create state in aggregate context.  It'll stay there across subsequent
+	 * calls.
+	 */
 	oldcontext = MemoryContextSwitchTo(aggcontext);
 	state = makeStringInfo();
 	MemoryContextSwitchTo(oldcontext);
@@ -3360,11 +3359,11 @@ string_agg_transfn(PG_FUNCTION_ARGS)
 
 	state = PG_ARGISNULL(0) ? NULL : (StringInfo) PG_GETARG_POINTER(0);
 
-	/* Append the element unless not null. */
+	/* Append the element unless null. */
 	if (!PG_ARGISNULL(1))
 	{
 		if (state == NULL)
-			state = makeStringAggState(fcinfo->context);
+			state = makeStringAggState(fcinfo);
 		appendStringInfoText(state, PG_GETARG_TEXT_PP(1));	/* value */
 	}
 
@@ -3382,11 +3381,12 @@ string_agg_delim_transfn(PG_FUNCTION_ARGS)
 
 	state = PG_ARGISNULL(0) ? NULL : (StringInfo) PG_GETARG_POINTER(0);
 
-	/* Append the value unless not null. */
+	/* Append the value unless null. */
 	if (!PG_ARGISNULL(1))
 	{
+		/* On the first time through, we ignore the delimiter. */
 		if (state == NULL)
-			state = makeStringAggState(fcinfo->context);
+			state = makeStringAggState(fcinfo);
 		else if (!PG_ARGISNULL(2))
 			appendStringInfoText(state, PG_GETARG_TEXT_PP(2));	/* delimiter */
 
@@ -3405,15 +3405,11 @@ string_agg_finalfn(PG_FUNCTION_ARGS)
 {
 	StringInfo		state;
 
-	if (PG_ARGISNULL(0))
-		PG_RETURN_NULL();
-
 	/* cannot be called directly because of internal-type argument */
-	Assert(fcinfo->context &&
+	Assert(AggCheckCallContext(fcinfo, NULL));
-		   (IsA(fcinfo->context, AggState) ||
+
-			IsA(fcinfo->context, WindowAggState)));
+	state = PG_ARGISNULL(0) ? NULL : (StringInfo) PG_GETARG_POINTER(0);
 
-	state = (StringInfo) PG_GETARG_POINTER(0);
 	if (state != NULL)
 		PG_RETURN_TEXT_P(cstring_to_text(state->data));
 	else

src/include/fmgr.h

@@ -11,7 +11,7 @@
  * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/fmgr.h,v 1.63 2010/01/02 16:58:00 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/fmgr.h,v 1.64 2010/02/08 20:39:52 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -530,6 +530,20 @@ extern PGFunction lookup_external_function(void *filehandle, char *funcname);
 extern void load_file(const char *filename, bool restricted);
 extern void **find_rendezvous_variable(const char *varName);
 
+/*
+ * Support for aggregate functions
+ *
+ * This is actually in executor/nodeAgg.c, but we declare it here since the
+ * whole point is for callers of it to not be overly friendly with nodeAgg.
+ */
+
+/* AggCheckCallContext can return one of the following codes, or 0: */
+#define AGG_CONTEXT_AGGREGATE	1			/* regular aggregate */
+#define AGG_CONTEXT_WINDOW		2			/* window function */
+
+extern int	AggCheckCallContext(FunctionCallInfo fcinfo,
+								MemoryContext *aggcontext);
+
 
 /*
  * !!! OLD INTERFACE !!!