Skip to content

Commit bd1693e

Browse files
tglsfdctglsfdc
committed
Fix small memory leak in partial-aggregate deserialization functions.
A deserialize function's result is short-lived data during partial aggregation, since we're just going to pass it to the combine function and then it's of no use anymore. However, the built-in deserialize functions allocated their results in the aggregate state context, resulting in a query-lifespan memory leak. It's probably not possible for this to amount to anything much at present, since the number of leaked results would only be the number of worker processes. But it might become a problem in future. To fix, don't use the same convenience subroutine for setting up results that the aggregate transition functions use. David Rowley Report: <10050.1466637736@sss.pgh.pa.us>
1 parent 2d67342 commit bd1693e

File tree

1 file changed

+45
-11
lines changed

1 file changed

+45
-11
lines changed

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

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3176,6 +3176,22 @@ makeNumericAggState(FunctionCallInfo fcinfo, bool calcSumX2)
31763176
return state;
31773177
}
31783178

3179+
/*
3180+
* Like makeNumericAggState(), but allocate the state in the current memory
3181+
* context.
3182+
*/
3183+
static NumericAggState *
3184+
makeNumericAggStateCurrentContext(bool calcSumX2)
3185+
{
3186+
NumericAggState *state;
3187+
3188+
state = (NumericAggState *) palloc0(sizeof(NumericAggState));
3189+
state->calcSumX2 = calcSumX2;
3190+
state->agg_context = CurrentMemoryContext;
3191+
3192+
return state;
3193+
}
3194+
31793195
/*
31803196
* Accumulate a new input value for numeric aggregate functions.
31813197
*/
@@ -3374,7 +3390,7 @@ numeric_combine(PG_FUNCTION_ARGS)
33743390
{
33753391
old_context = MemoryContextSwitchTo(agg_context);
33763392

3377-
state1 = makeNumericAggState(fcinfo, true);
3393+
state1 = makeNumericAggStateCurrentContext(true);
33783394
state1->N = state2->N;
33793395
state1->NaNcount = state2->NaNcount;
33803396
state1->maxScale = state2->maxScale;
@@ -3465,7 +3481,7 @@ numeric_avg_combine(PG_FUNCTION_ARGS)
34653481
{
34663482
old_context = MemoryContextSwitchTo(agg_context);
34673483

3468-
state1 = makeNumericAggState(fcinfo, false);
3484+
state1 = makeNumericAggStateCurrentContext(false);
34693485
state1->N = state2->N;
34703486
state1->NaNcount = state2->NaNcount;
34713487
state1->maxScale = state2->maxScale;
@@ -3579,12 +3595,12 @@ numeric_avg_deserialize(PG_FUNCTION_ARGS)
35793595

35803596
/*
35813597
* Copy the bytea into a StringInfo so that we can "receive" it using the
3582-
* standard pq API.
3598+
* standard recv-function infrastructure.
35833599
*/
35843600
initStringInfo(&buf);
35853601
appendBinaryStringInfo(&buf, VARDATA(sstate), VARSIZE(sstate) - VARHDRSZ);
35863602

3587-
result = makeNumericAggState(fcinfo, false);
3603+
result = makeNumericAggStateCurrentContext(false);
35883604

35893605
/* N */
35903606
result->N = pq_getmsgint64(&buf);
@@ -3691,12 +3707,12 @@ numeric_deserialize(PG_FUNCTION_ARGS)
36913707

36923708
/*
36933709
* Copy the bytea into a StringInfo so that we can "receive" it using the
3694-
* standard pq API.
3710+
* standard recv-function infrastructure.
36953711
*/
36963712
initStringInfo(&buf);
36973713
appendBinaryStringInfo(&buf, VARDATA(sstate), VARSIZE(sstate) - VARHDRSZ);
36983714

3699-
result = makeNumericAggState(fcinfo, false);
3715+
result = makeNumericAggStateCurrentContext(false);
37003716

37013717
/* N */
37023718
result->N = pq_getmsgint64(&buf);
@@ -3803,6 +3819,21 @@ makeInt128AggState(FunctionCallInfo fcinfo, bool calcSumX2)
38033819
return state;
38043820
}
38053821

3822+
/*
3823+
* Like makeInt128AggState(), but allocate the state in the current memory
3824+
* context.
3825+
*/
3826+
static Int128AggState *
3827+
makeInt128AggStateCurrentContext(bool calcSumX2)
3828+
{
3829+
Int128AggState *state;
3830+
3831+
state = (Int128AggState *) palloc0(sizeof(Int128AggState));
3832+
state->calcSumX2 = calcSumX2;
3833+
3834+
return state;
3835+
}
3836+
38063837
/*
38073838
* Accumulate a new input value for 128-bit aggregate functions.
38083839
*/
@@ -3831,9 +3862,11 @@ do_int128_discard(Int128AggState *state, int128 newval)
38313862

38323863
typedef Int128AggState PolyNumAggState;
38333864
#define makePolyNumAggState makeInt128AggState
3865+
#define makePolyNumAggStateCurrentContext makeInt128AggStateCurrentContext
38343866
#else
38353867
typedef NumericAggState PolyNumAggState;
38363868
#define makePolyNumAggState makeNumericAggState
3869+
#define makePolyNumAggStateCurrentContext makeNumericAggStateCurrentContext
38373870
#endif
38383871

38393872
Datum
@@ -4072,12 +4105,12 @@ numeric_poly_deserialize(PG_FUNCTION_ARGS)
40724105

40734106
/*
40744107
* Copy the bytea into a StringInfo so that we can "receive" it using the
4075-
* standard pq API.
4108+
* standard recv-function infrastructure.
40764109
*/
40774110
initStringInfo(&buf);
40784111
appendBinaryStringInfo(&buf, VARDATA(sstate), VARSIZE(sstate) - VARHDRSZ);
40794112

4080-
result = makePolyNumAggState(fcinfo, false);
4113+
result = makePolyNumAggStateCurrentContext(false);
40814114

40824115
/* N */
40834116
result->N = pq_getmsgint64(&buf);
@@ -4210,7 +4243,8 @@ int8_avg_combine(PG_FUNCTION_ARGS)
42104243

42114244
/*
42124245
* int8_avg_serialize
4213-
* Serialize PolyNumAggState into bytea using the standard pq API.
4246+
* Serialize PolyNumAggState into bytea using the standard
4247+
* recv-function infrastructure.
42144248
*/
42154249
Datum
42164250
int8_avg_serialize(PG_FUNCTION_ARGS)
@@ -4284,12 +4318,12 @@ int8_avg_deserialize(PG_FUNCTION_ARGS)
42844318

42854319
/*
42864320
* Copy the bytea into a StringInfo so that we can "receive" it using the
4287-
* standard pq API.
4321+
* standard recv-function infrastructure.
42884322
*/
42894323
initStringInfo(&buf);
42904324
appendBinaryStringInfo(&buf, VARDATA(sstate), VARSIZE(sstate) - VARHDRSZ);
42914325

4292-
result = makePolyNumAggState(fcinfo, false);
4326+
result = makePolyNumAggStateCurrentContext(false);
42934327

42944328
/* N */
42954329
result->N = pq_getmsgint64(&buf);

0 commit comments

Comments
 (0)