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

Commit96cfc7e

Browse files
committed
Prevent sharing transition states between ordered-set aggregates.
This ought to work, but the built-in OSAs are not capable of coping,because their final-functions destructively modify their transitionstate (specifically, the contained tuplesort object). That was finewhen those functions were written, but commit804163b moved thegoalposts without telling orderedsetaggs.c.We should fix the built-in OSAs to support this, but it will takea little work, especially if we don't want to sacrifice performancein the normal non-shared-state case. Given that it took a year after9.6 release for anyone to notice this bug, we should not prioritizesharable-state over nonsharable-state performance. And a proper fixis likely to be more complicated than we'd want to back-patch, too.Therefore, let's just put in this stop-gap patch to prevent nodeAgg.cfrom choosing to use shared state for OSAs. We can revert it in HEADwhen we get a better fix.Report from Lukas Eder, diagnosis by me, patch by David Rowley.Back-patch to 9.6 where the problem was introduced.Discussion:https://postgr.es/m/CAB4ELO5RZhOamuT9Xsf72ozbenDLLXZKSk07FiSVsuJNZB861A@mail.gmail.com
1 parent0da46d7 commit96cfc7e

File tree

3 files changed

+40
-0
lines changed

3 files changed

+40
-0
lines changed

‎src/backend/executor/nodeAgg.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3372,6 +3372,16 @@ find_compatible_pertrans(AggState *aggstate, Aggref *newagg,
33723372
{
33733373
ListCell*lc;
33743374

3375+
/*
3376+
* For the moment, never try to share transition states between different
3377+
* ordered-set aggregates. This is necessary because the finalfns of the
3378+
* built-in OSAs (see orderedsetaggs.c) are destructive of their
3379+
* transition states. We should fix them so we can allow this, but not
3380+
* losing performance in the normal non-shared case will take some work.
3381+
*/
3382+
if (AGGKIND_IS_ORDERED_SET(newagg->aggkind))
3383+
return-1;
3384+
33753385
foreach(lc,transnos)
33763386
{
33773387
inttransno=lfirst_int(lc);

‎src/test/regress/expected/aggregates.out

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1857,6 +1857,25 @@ NOTICE: avg_transfn called with 3
18571857
2 | 6
18581858
(1 row)
18591859

1860+
-- ideally these would share state, but we have to fix the OSAs first.
1861+
select
1862+
percentile_cont(0.5) within group (order by a),
1863+
percentile_disc(0.5) within group (order by a)
1864+
from (values(1::float8),(3),(5),(7)) t(a);
1865+
percentile_cont | percentile_disc
1866+
-----------------+-----------------
1867+
4 | 3
1868+
(1 row)
1869+
1870+
select
1871+
rank(4) within group (order by a),
1872+
dense_rank(4) within group (order by a)
1873+
from (values(1),(3),(5),(7)) t(a);
1874+
rank | dense_rank
1875+
------+------------
1876+
3 | 3
1877+
(1 row)
1878+
18601879
-- test that aggs with the same sfunc and initcond share the same agg state
18611880
create aggregate my_sum_init(int4)
18621881
(

‎src/test/regress/sql/aggregates.sql

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -739,6 +739,17 @@ select my_avg(one) filter (where one > 1),my_sum(one) from (values(1),(3)) t(one
739739
-- this should not share the state due to different input columns.
740740
select my_avg(one),my_sum(two)from (values(1,2),(3,4)) t(one,two);
741741

742+
-- ideally these would share state, but we have to fix the OSAs first.
743+
select
744+
percentile_cont(0.5) within group (order by a),
745+
percentile_disc(0.5) within group (order by a)
746+
from (values(1::float8),(3),(5),(7)) t(a);
747+
748+
select
749+
rank(4) within group (order by a),
750+
dense_rank(4) within group (order by a)
751+
from (values(1),(3),(5),(7)) t(a);
752+
742753
-- test that aggs with the same sfunc and initcond share the same agg state
743754
createaggregatemy_sum_init(int4)
744755
(

0 commit comments

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