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

Commitaf3deff

Browse files
committed
Fix slot type handling for Agg nodes performing internal sorts.
Since15d8f83 we assert that - and since7ef04e4,4da597erely on - the slot type for an expression'secxt_{outer,inner,scan}tuple not changing, unless explicitly flaggedas such. That allows to either skip deforming (for a virtual tupleslot) or optimize the code for JIT accelerated deformingappropriately (for other known slot types).This assumption was sometimes violated for grouping sets, whennodeAgg.c internally uses tuplesorts, and the child node doesn'treturn a TTSOpsMinimalTuple type slot. Detect that case, and flag thatthe outer slot might not be "fixed".It's probably worthwhile to optimize this further in the future, andmore granularly determine whether the slot is fixed. As we alreadyinstantiate per-phase transition and equal expressions, we couldcheaply set the slot type appropriately for each phase. But that's aseparate change from this bugfix.This commit does include a very minor optimization by avoiding tocreate a slot for handling tuplesorts, if no such sorts areperformed. Previously we created that slot unnecessarily in the commoncase of computing all grouping sets via hashing. The code looked tooconfusing without that, as the conditions for needing a sort slot andflagging that the slot type isn't fixed, are the same.Reported-By: Ashutosh SharmaAuthor: Andres FreundDiscussion:https://postgr.es/m/CAE9k0PmNaMD2oHTEAhRyxnxpaDaYkuBYkLa1dpOpn=RS0iS2AQ@mail.gmail.comBackpatch: 12-, where the bug was introduced in15d8f83
1 parentcb9bb15 commitaf3deff

File tree

3 files changed

+99
-2
lines changed

3 files changed

+99
-2
lines changed

‎src/backend/executor/nodeAgg.c

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2231,13 +2231,43 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
22312231
/*
22322232
* initialize source tuple type.
22332233
*/
2234+
aggstate->ss.ps.outerops=
2235+
ExecGetResultSlotOps(outerPlanState(&aggstate->ss),
2236+
&aggstate->ss.ps.outeropsfixed);
2237+
aggstate->ss.ps.outeropsset= true;
2238+
22342239
ExecCreateScanSlotFromOuterPlan(estate,&aggstate->ss,
2235-
ExecGetResultSlotOps(outerPlanState(&aggstate->ss),NULL));
2240+
aggstate->ss.ps.outerops);
22362241
scanDesc=aggstate->ss.ss_ScanTupleSlot->tts_tupleDescriptor;
2237-
if (node->chain)
2242+
2243+
/*
2244+
* If there are more than two phases (including a potential dummy phase
2245+
* 0), input will be resorted using tuplesort. Need a slot for that.
2246+
*/
2247+
if (numPhases>2)
2248+
{
22382249
aggstate->sort_slot=ExecInitExtraTupleSlot(estate,scanDesc,
22392250
&TTSOpsMinimalTuple);
22402251

2252+
/*
2253+
* The output of the tuplesort, and the output from the outer child
2254+
* might not use the same type of slot. In most cases the child will
2255+
* be a Sort, and thus return a TTSOpsMinimalTuple type slot - but the
2256+
* input can also be be presorted due an index, in which case it could
2257+
* be a different type of slot.
2258+
*
2259+
* XXX: For efficiency it would be good to instead/additionally
2260+
* generate expressions with corresponding settings of outerops* for
2261+
* the individual phases - deforming is often a bottleneck for
2262+
* aggregations with lots of rows per group. If there's multiple
2263+
* sorts, we know that all but the first use TTSOpsMinimalTuple (via
2264+
* the nodeAgg.c internal tuplesort).
2265+
*/
2266+
if (aggstate->ss.ps.outeropsfixed&&
2267+
aggstate->ss.ps.outerops!=&TTSOpsMinimalTuple)
2268+
aggstate->ss.ps.outeropsfixed= false;
2269+
}
2270+
22412271
/*
22422272
* Initialize result type, slot and projection.
22432273
*/

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1360,6 +1360,61 @@ explain (costs off)
13601360
-> Function Scan on gstest_data
13611361
(10 rows)
13621362

1363+
-- Verify that we correctly handle the child node returning a
1364+
-- non-minimal slot, which happens if the input is pre-sorted,
1365+
-- e.g. due to an index scan.
1366+
BEGIN;
1367+
SET LOCAL enable_hashagg = false;
1368+
EXPLAIN SELECT a, b, count(*), max(a), max(b) FROM gstest3 GROUP BY GROUPING SETS(a, b,()) ORDER BY a, b;
1369+
QUERY PLAN
1370+
-------------------------------------------------------------------------
1371+
Sort (cost=1.20..1.21 rows=5 width=24)
1372+
Sort Key: a, b
1373+
-> GroupAggregate (cost=1.03..1.14 rows=5 width=24)
1374+
Group Key: a
1375+
Group Key: ()
1376+
Sort Key: b
1377+
Group Key: b
1378+
-> Sort (cost=1.03..1.03 rows=2 width=8)
1379+
Sort Key: a
1380+
-> Seq Scan on gstest3 (cost=0.00..1.02 rows=2 width=8)
1381+
(10 rows)
1382+
1383+
SELECT a, b, count(*), max(a), max(b) FROM gstest3 GROUP BY GROUPING SETS(a, b,()) ORDER BY a, b;
1384+
a | b | count | max | max
1385+
---+---+-------+-----+-----
1386+
1 | | 1 | 1 | 1
1387+
2 | | 1 | 2 | 2
1388+
| 1 | 1 | 1 | 1
1389+
| 2 | 1 | 2 | 2
1390+
| | 2 | 2 | 2
1391+
(5 rows)
1392+
1393+
SET LOCAL enable_seqscan = false;
1394+
EXPLAIN SELECT a, b, count(*), max(a), max(b) FROM gstest3 GROUP BY GROUPING SETS(a, b,()) ORDER BY a, b;
1395+
QUERY PLAN
1396+
-----------------------------------------------------------------------------------------
1397+
Sort (cost=12.32..12.33 rows=5 width=24)
1398+
Sort Key: a, b
1399+
-> GroupAggregate (cost=0.13..12.26 rows=5 width=24)
1400+
Group Key: a
1401+
Group Key: ()
1402+
Sort Key: b
1403+
Group Key: b
1404+
-> Index Scan using gstest3_pkey on gstest3 (cost=0.13..12.16 rows=2 width=8)
1405+
(8 rows)
1406+
1407+
SELECT a, b, count(*), max(a), max(b) FROM gstest3 GROUP BY GROUPING SETS(a, b,()) ORDER BY a, b;
1408+
a | b | count | max | max
1409+
---+---+-------+-----+-----
1410+
1 | | 1 | 1 | 1
1411+
2 | | 1 | 2 | 2
1412+
| 1 | 1 | 1 | 1
1413+
| 2 | 1 | 2 | 2
1414+
| | 2 | 2 | 2
1415+
(5 rows)
1416+
1417+
COMMIT;
13631418
-- More rescan tests
13641419
select * from (values (1),(2)) v(a) left join lateral (select v.a, four, ten, count(*) from onek group by cube(four,ten)) s on true order by v.a,four,ten;
13651420
a | a | four | ten | count

‎src/test/regress/sql/groupingsets.sql

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,18 @@ explain (costs off)
384384
from (values (1),(2)) v(x), gstest_data(v.x)
385385
group by cube (a,b)order by a,b;
386386

387+
-- Verify that we correctly handle the child node returning a
388+
-- non-minimal slot, which happens if the input is pre-sorted,
389+
-- e.g. due to an index scan.
390+
BEGIN;
391+
SET LOCAL enable_hashagg= false;
392+
EXPLAINSELECT a, b,count(*),max(a),max(b)FROM gstest3GROUP BY GROUPING SETS(a, b,())ORDER BY a, b;
393+
SELECT a, b,count(*),max(a),max(b)FROM gstest3GROUP BY GROUPING SETS(a, b,())ORDER BY a, b;
394+
SET LOCAL enable_seqscan= false;
395+
EXPLAINSELECT a, b,count(*),max(a),max(b)FROM gstest3GROUP BY GROUPING SETS(a, b,())ORDER BY a, b;
396+
SELECT a, b,count(*),max(a),max(b)FROM gstest3GROUP BY GROUPING SETS(a, b,())ORDER BY a, b;
397+
COMMIT;
398+
387399
-- More rescan tests
388400
select*from (values (1),(2)) v(a)left join lateral (selectv.a, four, ten,count(*)from onekgroup by cube(four,ten)) son trueorder byv.a,four,ten;
389401
select array(select row(v.a,s1.*)from (select two,four,count(*)from onekgroup by cube(two,four)order by two,four) s1)from (values (1),(2)) v(a);

0 commit comments

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