Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit82480e2

Browse files
committed
Fix things so that array_agg_finalfn does not modify or free its input
ArrayBuildState, per trouble report from Merlin Moncure. By adoptingthis fix, we are essentially deciding that aggregate final-functionsshould not modify their inputs ever. Adjust documentation and commentsto match that conclusion.
1 parent87698ff commit82480e2

File tree

4 files changed

+31
-13
lines changed

4 files changed

+31
-13
lines changed

‎doc/src/sgml/xaggr.sgml

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!-- $PostgreSQL: pgsql/doc/src/sgml/xaggr.sgml,v 1.37 2008/12/28 18:53:54 tgl Exp $ -->
1+
<!-- $PostgreSQL: pgsql/doc/src/sgml/xaggr.sgml,v 1.38 2009/06/20 18:45:28 tgl Exp $ -->
22

33
<sect1 id="xaggr">
44
<title>User-Defined Aggregates</title>
@@ -175,10 +175,14 @@ SELECT attrelid::regclass, array_accum(atttypid::regtype)
175175
(IsA(fcinfo-&gt;context, AggState) ||
176176
IsA(fcinfo-&gt;context, WindowAggState)))
177177
</programlisting>
178-
One reason for checking this is that when it is true, the first input
178+
One reason for checking this is that when it is true for a transition
179+
function, the first input
179180
must be a temporary transition value and can therefore safely be modified
180181
in-place rather than allocating a new copy. (This is the <emphasis>only</>
181-
case where it is safe for a function to modify a pass-by-reference input.)
182+
case where it is safe for a function to modify a pass-by-reference input.
183+
In particular, aggregate final functions should not modify their inputs in
184+
any case, because in some cases they will be re-executed on the same
185+
final transition value.)
182186
See <literal>int8inc()</> for an example.
183187
</para>
184188

‎src/backend/executor/nodeWindowAgg.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
* Portions Copyright (c) 1994, Regents of the University of California
2828
*
2929
* IDENTIFICATION
30-
* $PostgreSQL: pgsql/src/backend/executor/nodeWindowAgg.c,v 1.5 2009/06/11 14:48:57 momjian Exp $
30+
* $PostgreSQL: pgsql/src/backend/executor/nodeWindowAgg.c,v 1.6 2009/06/20 18:45:28 tgl Exp $
3131
*
3232
*-------------------------------------------------------------------------
3333
*/
@@ -410,9 +410,8 @@ eval_windowaggregates(WindowAggState *winstate)
410410
* need the current aggregate value. This is considerably more efficient
411411
* than the naive approach of re-running the entire aggregate calculation
412412
* for each current row. It does assume that the final function doesn't
413-
* damage the running transition value. (Some C-coded aggregates do that
414-
* for efficiency's sake --- but they are supposed to do so only when
415-
* their fcinfo->context is an AggState, not a WindowAggState.)
413+
* damage the running transition value, but we have the same assumption
414+
* in nodeAgg.c too (when it rescans an existing hash table).
416415
*
417416
* In many common cases, multiple rows share the same frame and hence the
418417
* same aggregate value. (In particular, if there's no ORDER BY in a RANGE

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* Copyright (c) 2003-2009, PostgreSQL Global Development Group
77
*
88
* IDENTIFICATION
9-
* $PostgreSQL: pgsql/src/backend/utils/adt/array_userfuncs.c,v 1.30 2009/06/11 14:49:03 momjian Exp $
9+
* $PostgreSQL: pgsql/src/backend/utils/adt/array_userfuncs.c,v 1.31 2009/06/20 18:45:28 tgl Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
@@ -537,10 +537,13 @@ array_agg_finalfn(PG_FUNCTION_ARGS)
537537
dims[0]=state->nelems;
538538
lbs[0]=1;
539539

540-
/* Release working state if regular aggregate, but not if window agg */
540+
/*
541+
* Make the result. We cannot release the ArrayBuildState because
542+
* sometimes aggregate final functions are re-executed.
543+
*/
541544
result=makeMdArrayResult(state,1,dims,lbs,
542545
CurrentMemoryContext,
543-
IsA(fcinfo->context,AggState));
546+
false);
544547

545548
PG_RETURN_DATUM(result);
546549
}

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/utils/adt/arrayfuncs.c,v 1.157 2009/06/11 14:49:03 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/utils/adt/arrayfuncs.c,v 1.158 2009/06/20 18:45:28 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -4179,9 +4179,21 @@ accumArrayResult(ArrayBuildState *astate,
41794179
}
41804180
}
41814181

4182-
/* Use datumCopy to ensure pass-by-ref stuff is copied into mcontext */
4182+
/*
4183+
* Ensure pass-by-ref stuff is copied into mcontext; and detoast it too
4184+
* if it's varlena. (You might think that detoasting is not needed here
4185+
* because construct_md_array can detoast the array elements later.
4186+
* However, we must not let construct_md_array modify the ArrayBuildState
4187+
* because that would mean array_agg_finalfn damages its input, which
4188+
* is verboten. Also, this way frequently saves one copying step.)
4189+
*/
41834190
if (!disnull&& !astate->typbyval)
4184-
dvalue=datumCopy(dvalue,astate->typbyval,astate->typlen);
4191+
{
4192+
if (astate->typlen==-1)
4193+
dvalue=PointerGetDatum(PG_DETOAST_DATUM_COPY(dvalue));
4194+
else
4195+
dvalue=datumCopy(dvalue,astate->typbyval,astate->typlen);
4196+
}
41854197

41864198
astate->dvalues[astate->nelems]=dvalue;
41874199
astate->dnulls[astate->nelems]=disnull;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp