Skip to content

Commit d5768dc

Browse files
tglsfdctglsfdc
committed
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.
1 parent 4d3d2e2 commit d5768dc

File tree

9 files changed

+124
-110
lines changed

9 files changed

+124
-110
lines changed

‎contrib/tsearch2/tsearch2.c

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $PostgreSQL: pgsql/contrib/tsearch2/tsearch2.c,v 1.11 2010/01/02 16:57:32 momjian Exp $
10+
* $PostgreSQL: pgsql/contrib/tsearch2/tsearch2.c,v 1.12 2010/02/08 20:39:51 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -422,15 +422,8 @@ tsa_rewrite_accum(PG_FUNCTION_ARGS)
422422
MemoryContext aggcontext;
423423
MemoryContext oldcontext;
424424

425-
if (fcinfo->context && IsA(fcinfo->context, AggState))
426-
aggcontext = ((AggState *) fcinfo->context)->aggcontext;
427-
else if (fcinfo->context && IsA(fcinfo->context, WindowAggState))
428-
aggcontext = ((WindowAggState *) fcinfo->context)->wincontext;
429-
else
430-
{
425+
if (!AggCheckCallContext(fcinfo, &aggcontext))
431426
elog(ERROR, "tsa_rewrite_accum called in non-aggregate context");
432-
aggcontext = NULL; /* keep compiler quiet */
433-
}
434427

435428
if (PG_ARGISNULL(0) || PG_GETARG_POINTER(0) == NULL)
436429
{

‎doc/src/sgml/xaggr.sgml

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!-- $PostgreSQL: pgsql/doc/src/sgml/xaggr.sgml,v 1.38 2009/06/20 18:45:28 tgl Exp $ -->
1+
<!-- $PostgreSQL: pgsql/doc/src/sgml/xaggr.sgml,v 1.39 2010/02/08 20:39:51 tgl Exp $ -->
22

33
<sect1 id="xaggr">
44
<title>User-Defined Aggregates</title>
@@ -166,14 +166,10 @@ SELECT attrelid::regclass, array_accum(atttypid::regtype)
166166

167167
<para>
168168
A function written in C can detect that it is being called as an
169-
aggregate transition or final function by seeing if it was passed
170-
an <structname>AggState</> or <structname>WindowAggState</> node
171-
as the function call <quote>context</>,
172-
for example by:
169+
aggregate transition or final function by calling
170+
<function>AggCheckCallContext</>, for example:
173171
<programlisting>
174-
if (fcinfo-&gt;context &amp;&amp;
175-
(IsA(fcinfo-&gt;context, AggState) ||
176-
IsA(fcinfo-&gt;context, WindowAggState)))
172+
if (AggCheckCallContext(fcinfo, NULL))
177173
</programlisting>
178174
One reason for checking this is that when it is true for a transition
179175
function, the first input

‎src/backend/executor/nodeAgg.c

Lines changed: 58 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -44,30 +44,34 @@
4444
* is used to run finalize functions and compute the output tuple;
4545
* this context can be reset once per output tuple.
4646
*
47-
* Beginning in PostgreSQL 8.1, the executor's AggState node is passed as
48-
* the fmgr "context" value in all transfunc and finalfunc calls. It is
49-
* not really intended that the transition functions will look into the
50-
* AggState node, but they can use code like
51-
* if (fcinfo->context && IsA(fcinfo->context, AggState))
52-
* to verify that they are being called by nodeAgg.c and not as ordinary
53-
* SQL functions. The main reason a transition function might want to know
54-
* that is that it can avoid palloc'ing a fixed-size pass-by-ref transition
55-
* value on every call: it can instead just scribble on and return its left
56-
* input. Ordinarily it is completely forbidden for functions to modify
57-
* pass-by-ref inputs, but in the aggregate case we know the left input is
58-
* either the initial transition value or a previous function result, and
59-
* in either case its value need not be preserved. See int8inc() for an
60-
* example. Notice that advance_transition_function() is coded to avoid a
61-
* data copy step when the previous transition value pointer is returned.
62-
* Also, some transition functions make use of the aggcontext to store
63-
* working state.
47+
* The executor's AggState node is passed as the fmgr "context" value in
48+
* all transfunc and finalfunc calls. It is not recommended that the
49+
* transition functions look at the AggState node directly, but they can
50+
* use AggCheckCallContext() to verify that they are being called by
51+
* nodeAgg.c (and not as ordinary SQL functions). The main reason a
52+
* transition function might want to know this is so that it can avoid
53+
* palloc'ing a fixed-size pass-by-ref transition value on every call:
54+
* it can instead just scribble on and return its left input. Ordinarily
55+
* it is completely forbidden for functions to modify pass-by-ref inputs,
56+
* but in the aggregate case we know the left input is either the initial
57+
* transition value or a previous function result, and in either case its
58+
* value need not be preserved. See int8inc() for an example. Notice that
59+
* advance_transition_function() is coded to avoid a data copy step when
60+
* the previous transition value pointer is returned. Also, some
61+
* transition functions want to store working state in addition to the
62+
* nominal transition value; they can use the memory context returned by
63+
* AggCheckCallContext() to do that.
64+
*
65+
* Note: AggCheckCallContext() is available as of PostgreSQL 9.0. The
66+
* AggState is available as context in earlier releases (back to 8.1),
67+
* but direct examination of the node is needed to use it before 9.0.
6468
*
6569
*
6670
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
6771
* Portions Copyright (c) 1994, Regents of the University of California
6872
*
6973
* IDENTIFICATION
70-
* $PostgreSQL: pgsql/src/backend/executor/nodeAgg.c,v 1.171 2010/01/02 16:57:41 momjian Exp $
74+
* $PostgreSQL: pgsql/src/backend/executor/nodeAgg.c,v 1.172 2010/02/08 20:39:51 tgl Exp $
7175
*
7276
*-------------------------------------------------------------------------
7377
*/
@@ -1969,6 +1973,42 @@ ExecReScanAgg(AggState *node, ExprContext *exprCtxt)
19691973
ExecReScan(((PlanState *) node)->lefttree, exprCtxt);
19701974
}
19711975

1976+
/*
1977+
* AggCheckCallContext - test if a SQL function is being called as an aggregate
1978+
*
1979+
* The transition and/or final functions of an aggregate may want to verify
1980+
* that they are being called as aggregates, rather than as plain SQL
1981+
* functions. They should use this function to do so. The return value
1982+
* is nonzero if being called as an aggregate, or zero if not. (Specific
1983+
* nonzero values are AGG_CONTEXT_AGGREGATE or AGG_CONTEXT_WINDOW, but more
1984+
* values could conceivably appear in future.)
1985+
*
1986+
* If aggcontext isn't NULL, the function also stores at *aggcontext the
1987+
* identity of the memory context that aggregate transition values are
1988+
* being stored in.
1989+
*/
1990+
int
1991+
AggCheckCallContext(FunctionCallInfo fcinfo, MemoryContext *aggcontext)
1992+
{
1993+
if (fcinfo->context && IsA(fcinfo->context, AggState))
1994+
{
1995+
if (aggcontext)
1996+
*aggcontext = ((AggState *) fcinfo->context)->aggcontext;
1997+
return AGG_CONTEXT_AGGREGATE;
1998+
}
1999+
if (fcinfo->context && IsA(fcinfo->context, WindowAggState))
2000+
{
2001+
if (aggcontext)
2002+
*aggcontext = ((WindowAggState *) fcinfo->context)->wincontext;
2003+
return AGG_CONTEXT_WINDOW;
2004+
}
2005+
2006+
/* this is just to prevent "uninitialized variable" warnings */
2007+
if (aggcontext)
2008+
*aggcontext = NULL;
2009+
return 0;
2010+
}
2011+
19722012
/*
19732013
* aggregate_dummy - dummy execution routine for aggregate functions
19742014
*

‎src/backend/utils/adt/array_userfuncs.c

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,12 @@
66
* Copyright (c) 2003-2010, PostgreSQL Global Development Group
77
*
88
* IDENTIFICATION
9-
* $PostgreSQL: pgsql/src/backend/utils/adt/array_userfuncs.c,v 1.33 2010/01/02 16:57:53 momjian Exp $
9+
* $PostgreSQL: pgsql/src/backend/utils/adt/array_userfuncs.c,v 1.34 2010/02/08 20:39:51 tgl Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
1313
#include "postgres.h"
1414

15-
#include "nodes/execnodes.h"
1615
#include "utils/array.h"
1716
#include "utils/builtins.h"
1817
#include "utils/lsyscache.h"
@@ -484,15 +483,10 @@ array_agg_transfn(PG_FUNCTION_ARGS)
484483
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
485484
errmsg("could not determine input data type")));
486485

487-
if (fcinfo->context && IsA(fcinfo->context, AggState))
488-
aggcontext = ((AggState *) fcinfo->context)->aggcontext;
489-
else if (fcinfo->context && IsA(fcinfo->context, WindowAggState))
490-
aggcontext = ((WindowAggState *) fcinfo->context)->wincontext;
491-
else
486+
if (!AggCheckCallContext(fcinfo, &aggcontext))
492487
{
493488
/* cannot be called directly because of internal-type argument */
494489
elog(ERROR, "array_agg_transfn called in non-aggregate context");
495-
aggcontext = NULL; /* keep compiler quiet */
496490
}
497491

498492
state = PG_ARGISNULL(0) ? NULL : (ArrayBuildState *) PG_GETARG_POINTER(0);
@@ -528,9 +522,7 @@ array_agg_finalfn(PG_FUNCTION_ARGS)
528522
PG_RETURN_NULL(); /* returns null iff no input values */
529523

530524
/* cannot be called directly because of internal-type argument */
531-
Assert(fcinfo->context &&
532-
(IsA(fcinfo->context, AggState) ||
533-
IsA(fcinfo->context, WindowAggState)));
525+
Assert(AggCheckCallContext(fcinfo, NULL));
534526

535527
state = (ArrayBuildState *) PG_GETARG_POINTER(0);
536528

‎src/backend/utils/adt/float.c

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/utils/adt/float.c,v 1.164 2010/01/02 16:57:53 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/utils/adt/float.c,v 1.165 2010/02/08 20:39:51 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -1765,13 +1765,11 @@ float8_accum(PG_FUNCTION_ARGS)
17651765
CHECKFLOATVAL(sumX2, isinf(transvalues[2]) || isinf(newval), true);
17661766

17671767
/*
1768-
* If we're invoked by nodeAgg, we can cheat and modify our first
1768+
* If we're invoked as an aggregate, we can cheat and modify our first
17691769
* parameter in-place to reduce palloc overhead. Otherwise we construct a
17701770
* new array with the updated transition data and return it.
17711771
*/
1772-
if (fcinfo->context &&
1773-
(IsA(fcinfo->context, AggState) ||
1774-
IsA(fcinfo->context, WindowAggState)))
1772+
if (AggCheckCallContext(fcinfo, NULL))
17751773
{
17761774
transvalues[0] = N;
17771775
transvalues[1] = sumX;
@@ -1820,13 +1818,11 @@ float4_accum(PG_FUNCTION_ARGS)
18201818
CHECKFLOATVAL(sumX2, isinf(transvalues[2]) || isinf(newval), true);
18211819

18221820
/*
1823-
* If we're invoked by nodeAgg, we can cheat and modify our first
1821+
* If we're invoked as an aggregate, we can cheat and modify our first
18241822
* parameter in-place to reduce palloc overhead. Otherwise we construct a
18251823
* new array with the updated transition data and return it.
18261824
*/
1827-
if (fcinfo->context &&
1828-
(IsA(fcinfo->context, AggState) ||
1829-
IsA(fcinfo->context, WindowAggState)))
1825+
if (AggCheckCallContext(fcinfo, NULL))
18301826
{
18311827
transvalues[0] = N;
18321828
transvalues[1] = sumX;
@@ -2039,13 +2035,11 @@ float8_regr_accum(PG_FUNCTION_ARGS)
20392035
isinf(newvalY), true);
20402036

20412037
/*
2042-
* If we're invoked by nodeAgg, we can cheat and modify our first
2038+
* If we're invoked as an aggregate, we can cheat and modify our first
20432039
* parameter in-place to reduce palloc overhead. Otherwise we construct a
20442040
* new array with the updated transition data and return it.
20452041
*/
2046-
if (fcinfo->context &&
2047-
(IsA(fcinfo->context, AggState) ||
2048-
IsA(fcinfo->context, WindowAggState)))
2042+
if (AggCheckCallContext(fcinfo, NULL))
20492043
{
20502044
transvalues[0] = N;
20512045
transvalues[1] = sumX;

‎src/backend/utils/adt/int8.c

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
99
* IDENTIFICATION
10-
* $PostgreSQL: pgsql/src/backend/utils/adt/int8.c,v 1.77 2010/01/07 04:53:34 tgl Exp $
10+
* $PostgreSQL: pgsql/src/backend/utils/adt/int8.c,v 1.78 2010/02/08 20:39:51 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -19,7 +19,6 @@
1919

2020
#include "funcapi.h"
2121
#include "libpq/pqformat.h"
22-
#include "nodes/nodes.h"
2322
#include "utils/int8.h"
2423

2524

@@ -654,15 +653,13 @@ int8inc(PG_FUNCTION_ARGS)
654653
{
655654
/*
656655
* When int8 is pass-by-reference, we provide this special case to avoid
657-
* palloc overhead for COUNT(): when called from nodeAgg, we know that the
658-
* argument is modifiable local storage, so just update it in-place. (If
659-
* int8 is pass-by-value, then of course this is useless as well as
660-
* incorrect, so just ifdef it out.)
656+
* palloc overhead for COUNT(): when called as an aggregate, we know that
657+
* the argument is modifiable local storage, so just update it
658+
* in-place. (If int8 is pass-by-value, then of course this is useless as
659+
* well as incorrect, so just ifdef it out.)
661660
*/
662661
#ifndef USE_FLOAT8_BYVAL /* controls int8 too */
663-
if (fcinfo->context &&
664-
(IsA(fcinfo->context, AggState) ||
665-
IsA(fcinfo->context, WindowAggState)))
662+
if (AggCheckCallContext(fcinfo, NULL))
666663
{
667664
int64 *arg = (int64 *) PG_GETARG_POINTER(0);
668665
int64 result;
@@ -680,7 +677,7 @@ int8inc(PG_FUNCTION_ARGS)
680677
else
681678
#endif
682679
{
683-
/* Not called by nodeAgg, so just do it the dumb way */
680+
/* Not called as an aggregate, so just do it the dumb way */
684681
int64 arg = PG_GETARG_INT64(0);
685682
int64 result;
686683

‎src/backend/utils/adt/numeric.c

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* Copyright (c) 1998-2010, PostgreSQL Global Development Group
1515
*
1616
* IDENTIFICATION
17-
* $PostgreSQL: pgsql/src/backend/utils/adt/numeric.c,v 1.121 2010/01/07 04:53:34 tgl Exp $
17+
* $PostgreSQL: pgsql/src/backend/utils/adt/numeric.c,v 1.122 2010/02/08 20:39:51 tgl Exp $
1818
*
1919
*-------------------------------------------------------------------------
2020
*/
@@ -2679,16 +2679,14 @@ int2_sum(PG_FUNCTION_ARGS)
26792679
}
26802680

26812681
/*
2682-
* If we're invoked by nodeAgg, we can cheat and modify our first
2682+
* If we're invoked as an aggregate, we can cheat and modify our first
26832683
* parameter in-place to avoid palloc overhead. If not, we need to return
26842684
* the new value of the transition variable. (If int8 is pass-by-value,
26852685
* then of course this is useless as well as incorrect, so just ifdef it
26862686
* out.)
26872687
*/
26882688
#ifndef USE_FLOAT8_BYVAL /* controls int8 too */
2689-
if (fcinfo->context &&
2690-
(IsA(fcinfo->context, AggState) ||
2691-
IsA(fcinfo->context, WindowAggState)))
2689+
if (AggCheckCallContext(fcinfo, NULL))
26922690
{
26932691
int64 *oldsum = (int64 *) PG_GETARG_POINTER(0);
26942692

@@ -2730,16 +2728,14 @@ int4_sum(PG_FUNCTION_ARGS)
27302728
}
27312729

27322730
/*
2733-
* If we're invoked by nodeAgg, we can cheat and modify our first
2731+
* If we're invoked as an aggregate, we can cheat and modify our first
27342732
* parameter in-place to avoid palloc overhead. If not, we need to return
27352733
* the new value of the transition variable. (If int8 is pass-by-value,
27362734
* then of course this is useless as well as incorrect, so just ifdef it
27372735
* out.)
27382736
*/
27392737
#ifndef USE_FLOAT8_BYVAL /* controls int8 too */
2740-
if (fcinfo->context &&
2741-
(IsA(fcinfo->context, AggState) ||
2742-
IsA(fcinfo->context, WindowAggState)))
2738+
if (AggCheckCallContext(fcinfo, NULL))
27432739
{
27442740
int64 *oldsum = (int64 *) PG_GETARG_POINTER(0);
27452741

@@ -2782,7 +2778,7 @@ int8_sum(PG_FUNCTION_ARGS)
27822778
}
27832779

27842780
/*
2785-
* Note that we cannot special-case the nodeAgg case here, as we do for
2781+
* Note that we cannot special-case the aggregate case here, as we do for
27862782
* int2_sum and int4_sum: numeric is of variable size, so we cannot modify
27872783
* our first parameter in-place.
27882784
*/
@@ -2820,13 +2816,11 @@ int2_avg_accum(PG_FUNCTION_ARGS)
28202816
Int8TransTypeData *transdata;
28212817

28222818
/*
2823-
* If we're invoked by nodeAgg, we can cheat and modify our first
2819+
* If we're invoked as an aggregate, we can cheat and modify our first
28242820
* parameter in-place to reduce palloc overhead. Otherwise we need to make
28252821
* a copy of it before scribbling on it.
28262822
*/
2827-
if (fcinfo->context &&
2828-
(IsA(fcinfo->context, AggState) ||
2829-
IsA(fcinfo->context, WindowAggState)))
2823+
if (AggCheckCallContext(fcinfo, NULL))
28302824
transarray = PG_GETARG_ARRAYTYPE_P(0);
28312825
else
28322826
transarray = PG_GETARG_ARRAYTYPE_P_COPY(0);
@@ -2850,13 +2844,11 @@ int4_avg_accum(PG_FUNCTION_ARGS)
28502844
Int8TransTypeData *transdata;
28512845

28522846
/*
2853-
* If we're invoked by nodeAgg, we can cheat and modify our first
2847+
* If we're invoked as an aggregate, we can cheat and modify our first
28542848
* parameter in-place to reduce palloc overhead. Otherwise we need to make
28552849
* a copy of it before scribbling on it.
28562850
*/
2857-
if (fcinfo->context &&
2858-
(IsA(fcinfo->context, AggState) ||
2859-
IsA(fcinfo->context, WindowAggState)))
2851+
if (AggCheckCallContext(fcinfo, NULL))
28602852
transarray = PG_GETARG_ARRAYTYPE_P(0);
28612853
else
28622854
transarray = PG_GETARG_ARRAYTYPE_P_COPY(0);

0 commit comments

Comments
 (0)