Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit ce92fc4

Browse files
committed
Fix sharing Agg transition state of DISTINCT or ordered aggs.
If a query contained two aggregates that could share the transition value, we would correctly collect the input into a tuplesort only once, but incorrectly run the transition function over the accumulated input twice, in finalize_aggregates(). That caused a crash, when we tried to call tuplesort_performsort() on an already-freed NULL tuplestore. Backport to 9.6, where sharing of transition state and this bug were introduced. Analysis by Tom Lane. Discussion: https://www.postgresql.org/message-id/ac5b0b69-744c-9114-6218-8300ac920e61@iki.fi
1 parent 3f07eff commit ce92fc4

File tree

3 files changed

+30
-3
lines changed

3 files changed

+30
-3
lines changed

src/backend/executor/nodeAgg.c

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1565,16 +1565,19 @@ finalize_aggregates(AggState *aggstate,
15651565
Datum *aggvalues = econtext->ecxt_aggvalues;
15661566
bool *aggnulls = econtext->ecxt_aggnulls;
15671567
int aggno;
1568+
int transno;
15681569

15691570
Assert(currentSet == 0 ||
15701571
((Agg *) aggstate->ss.ps.plan)->aggstrategy != AGG_HASHED);
15711572

15721573
aggstate->current_set = currentSet;
15731574

1574-
for (aggno = 0; aggno < aggstate->numaggs; aggno++)
1575+
/*
1576+
* If there were any DISTINCT and/or ORDER BY aggregates, sort their
1577+
* inputs and run the transition functions.
1578+
*/
1579+
for (transno = 0; transno < aggstate->numtrans; transno++)
15751580
{
1576-
AggStatePerAgg peragg = &peraggs[aggno];
1577-
int transno = peragg->transno;
15781581
AggStatePerTrans pertrans = &aggstate->pertrans[transno];
15791582
AggStatePerGroup pergroupstate;
15801583

@@ -1593,6 +1596,18 @@ finalize_aggregates(AggState *aggstate,
15931596
pertrans,
15941597
pergroupstate);
15951598
}
1599+
}
1600+
1601+
/*
1602+
* Run the final functions.
1603+
*/
1604+
for (aggno = 0; aggno < aggstate->numaggs; aggno++)
1605+
{
1606+
AggStatePerAgg peragg = &peraggs[aggno];
1607+
int transno = peragg->transno;
1608+
AggStatePerGroup pergroupstate;
1609+
1610+
pergroupstate = &pergroup[transno + (currentSet * (aggstate->numtrans))];
15961611

15971612
if (DO_AGGSPLIT_SKIPFINAL(aggstate->aggsplit))
15981613
finalize_partialaggregate(aggstate, peragg, pergroupstate,

src/test/regress/expected/aggregates.out

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1816,6 +1816,15 @@ NOTICE: avg_transfn called with 3
18161816
2 | 4
18171817
(1 row)
18181818

1819+
-- same as previous one, but with DISTINCT, which requires sorting the input.
1820+
select my_avg(distinct one),my_sum(distinct one) from (values(1),(3),(1)) t(one);
1821+
NOTICE: avg_transfn called with 1
1822+
NOTICE: avg_transfn called with 3
1823+
my_avg | my_sum
1824+
--------+--------
1825+
2 | 4
1826+
(1 row)
1827+
18191828
-- shouldn't share states due to the distinctness not matching.
18201829
select my_avg(distinct one),my_sum(one) from (values(1),(3)) t(one);
18211830
NOTICE: avg_transfn called with 1

src/test/regress/sql/aggregates.sql

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,9 @@ select my_avg(one),my_avg(one) from (values(1),(3)) t(one);
727727
-- aggregate state should be shared as transfn is the same for both aggs.
728728
select my_avg(one),my_sum(one) from (values(1),(3)) t(one);
729729

730+
-- same as previous one, but with DISTINCT, which requires sorting the input.
731+
select my_avg(distinct one),my_sum(distinct one) from (values(1),(3),(1)) t(one);
732+
730733
-- shouldn't share states due to the distinctness not matching.
731734
select my_avg(distinct one),my_sum(one) from (values(1),(3)) t(one);
732735

0 commit comments

Comments
 (0)