mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-09-06 23:29:20 +02:00
Fix slot type handling for Agg nodes performing internal sorts.
Since15d8f8312
we assert that - and since7ef04e4d2c
,4da597edf1
rely on - the slot type for an expression's ecxt_{outer,inner,scan}tuple not changing, unless explicitly flagged as such. That allows to either skip deforming (for a virtual tuple slot) or optimize the code for JIT accelerated deforming appropriately (for other known slot types). This assumption was sometimes violated for grouping sets, when nodeAgg.c internally uses tuplesorts, and the child node doesn't return a TTSOpsMinimalTuple type slot. Detect that case, and flag that the outer slot might not be "fixed". It's probably worthwhile to optimize this further in the future, and more granularly determine whether the slot is fixed. As we already instantiate per-phase transition and equal expressions, we could cheaply set the slot type appropriately for each phase. But that's a separate change from this bugfix. This commit does include a very minor optimization by avoiding to create a slot for handling tuplesorts, if no such sorts are performed. Previously we created that slot unnecessarily in the common case of computing all grouping sets via hashing. The code looked too confusing without that, as the conditions for needing a sort slot and flagging that the slot type isn't fixed, are the same. Reported-By: Ashutosh Sharma Author: Andres Freund Discussion: https://postgr.es/m/CAE9k0PmNaMD2oHTEAhRyxnxpaDaYkuBYkLa1dpOpn=RS0iS2AQ@mail.gmail.com Backpatch: 12-, where the bug was introduced in15d8f8312
This commit is contained in:
parent
b245370467
commit
8677c62eae
@ -2231,13 +2231,43 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
|
|||||||
/*
|
/*
|
||||||
* initialize source tuple type.
|
* initialize source tuple type.
|
||||||
*/
|
*/
|
||||||
|
aggstate->ss.ps.outerops =
|
||||||
|
ExecGetResultSlotOps(outerPlanState(&aggstate->ss),
|
||||||
|
&aggstate->ss.ps.outeropsfixed);
|
||||||
|
aggstate->ss.ps.outeropsset = true;
|
||||||
|
|
||||||
ExecCreateScanSlotFromOuterPlan(estate, &aggstate->ss,
|
ExecCreateScanSlotFromOuterPlan(estate, &aggstate->ss,
|
||||||
ExecGetResultSlotOps(outerPlanState(&aggstate->ss), NULL));
|
aggstate->ss.ps.outerops);
|
||||||
scanDesc = aggstate->ss.ss_ScanTupleSlot->tts_tupleDescriptor;
|
scanDesc = aggstate->ss.ss_ScanTupleSlot->tts_tupleDescriptor;
|
||||||
if (node->chain)
|
|
||||||
|
/*
|
||||||
|
* If there are more than two phases (including a potential dummy phase
|
||||||
|
* 0), input will be resorted using tuplesort. Need a slot for that.
|
||||||
|
*/
|
||||||
|
if (numPhases > 2)
|
||||||
|
{
|
||||||
aggstate->sort_slot = ExecInitExtraTupleSlot(estate, scanDesc,
|
aggstate->sort_slot = ExecInitExtraTupleSlot(estate, scanDesc,
|
||||||
&TTSOpsMinimalTuple);
|
&TTSOpsMinimalTuple);
|
||||||
|
|
||||||
|
/*
|
||||||
|
* The output of the tuplesort, and the output from the outer child
|
||||||
|
* might not use the same type of slot. In most cases the child will
|
||||||
|
* be a Sort, and thus return a TTSOpsMinimalTuple type slot - but the
|
||||||
|
* input can also be be presorted due an index, in which case it could
|
||||||
|
* be a different type of slot.
|
||||||
|
*
|
||||||
|
* XXX: For efficiency it would be good to instead/additionally
|
||||||
|
* generate expressions with corresponding settings of outerops* for
|
||||||
|
* the individual phases - deforming is often a bottleneck for
|
||||||
|
* aggregations with lots of rows per group. If there's multiple
|
||||||
|
* sorts, we know that all but the first use TTSOpsMinimalTuple (via
|
||||||
|
* the nodeAgg.c internal tuplesort).
|
||||||
|
*/
|
||||||
|
if (aggstate->ss.ps.outeropsfixed &&
|
||||||
|
aggstate->ss.ps.outerops != &TTSOpsMinimalTuple)
|
||||||
|
aggstate->ss.ps.outeropsfixed = false;
|
||||||
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Initialize result type, slot and projection.
|
* Initialize result type, slot and projection.
|
||||||
*/
|
*/
|
||||||
|
@ -1360,6 +1360,61 @@ explain (costs off)
|
|||||||
-> Function Scan on gstest_data
|
-> Function Scan on gstest_data
|
||||||
(10 rows)
|
(10 rows)
|
||||||
|
|
||||||
|
-- Verify that we correctly handle the child node returning a
|
||||||
|
-- non-minimal slot, which happens if the input is pre-sorted,
|
||||||
|
-- e.g. due to an index scan.
|
||||||
|
BEGIN;
|
||||||
|
SET LOCAL enable_hashagg = false;
|
||||||
|
EXPLAIN SELECT a, b, count(*), max(a), max(b) FROM gstest3 GROUP BY GROUPING SETS(a, b,()) ORDER BY a, b;
|
||||||
|
QUERY PLAN
|
||||||
|
-------------------------------------------------------------------------
|
||||||
|
Sort (cost=1.20..1.21 rows=5 width=24)
|
||||||
|
Sort Key: a, b
|
||||||
|
-> GroupAggregate (cost=1.03..1.14 rows=5 width=24)
|
||||||
|
Group Key: a
|
||||||
|
Group Key: ()
|
||||||
|
Sort Key: b
|
||||||
|
Group Key: b
|
||||||
|
-> Sort (cost=1.03..1.03 rows=2 width=8)
|
||||||
|
Sort Key: a
|
||||||
|
-> Seq Scan on gstest3 (cost=0.00..1.02 rows=2 width=8)
|
||||||
|
(10 rows)
|
||||||
|
|
||||||
|
SELECT a, b, count(*), max(a), max(b) FROM gstest3 GROUP BY GROUPING SETS(a, b,()) ORDER BY a, b;
|
||||||
|
a | b | count | max | max
|
||||||
|
---+---+-------+-----+-----
|
||||||
|
1 | | 1 | 1 | 1
|
||||||
|
2 | | 1 | 2 | 2
|
||||||
|
| 1 | 1 | 1 | 1
|
||||||
|
| 2 | 1 | 2 | 2
|
||||||
|
| | 2 | 2 | 2
|
||||||
|
(5 rows)
|
||||||
|
|
||||||
|
SET LOCAL enable_seqscan = false;
|
||||||
|
EXPLAIN SELECT a, b, count(*), max(a), max(b) FROM gstest3 GROUP BY GROUPING SETS(a, b,()) ORDER BY a, b;
|
||||||
|
QUERY PLAN
|
||||||
|
-----------------------------------------------------------------------------------------
|
||||||
|
Sort (cost=12.32..12.33 rows=5 width=24)
|
||||||
|
Sort Key: a, b
|
||||||
|
-> GroupAggregate (cost=0.13..12.26 rows=5 width=24)
|
||||||
|
Group Key: a
|
||||||
|
Group Key: ()
|
||||||
|
Sort Key: b
|
||||||
|
Group Key: b
|
||||||
|
-> Index Scan using gstest3_pkey on gstest3 (cost=0.13..12.16 rows=2 width=8)
|
||||||
|
(8 rows)
|
||||||
|
|
||||||
|
SELECT a, b, count(*), max(a), max(b) FROM gstest3 GROUP BY GROUPING SETS(a, b,()) ORDER BY a, b;
|
||||||
|
a | b | count | max | max
|
||||||
|
---+---+-------+-----+-----
|
||||||
|
1 | | 1 | 1 | 1
|
||||||
|
2 | | 1 | 2 | 2
|
||||||
|
| 1 | 1 | 1 | 1
|
||||||
|
| 2 | 1 | 2 | 2
|
||||||
|
| | 2 | 2 | 2
|
||||||
|
(5 rows)
|
||||||
|
|
||||||
|
COMMIT;
|
||||||
-- More rescan tests
|
-- More rescan tests
|
||||||
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;
|
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;
|
||||||
a | a | four | ten | count
|
a | a | four | ten | count
|
||||||
|
@ -384,6 +384,18 @@ explain (costs off)
|
|||||||
from (values (1),(2)) v(x), gstest_data(v.x)
|
from (values (1),(2)) v(x), gstest_data(v.x)
|
||||||
group by cube (a,b) order by a,b;
|
group by cube (a,b) order by a,b;
|
||||||
|
|
||||||
|
-- Verify that we correctly handle the child node returning a
|
||||||
|
-- non-minimal slot, which happens if the input is pre-sorted,
|
||||||
|
-- e.g. due to an index scan.
|
||||||
|
BEGIN;
|
||||||
|
SET LOCAL enable_hashagg = false;
|
||||||
|
EXPLAIN SELECT a, b, count(*), max(a), max(b) FROM gstest3 GROUP BY GROUPING SETS(a, b,()) ORDER BY a, b;
|
||||||
|
SELECT a, b, count(*), max(a), max(b) FROM gstest3 GROUP BY GROUPING SETS(a, b,()) ORDER BY a, b;
|
||||||
|
SET LOCAL enable_seqscan = false;
|
||||||
|
EXPLAIN SELECT a, b, count(*), max(a), max(b) FROM gstest3 GROUP BY GROUPING SETS(a, b,()) ORDER BY a, b;
|
||||||
|
SELECT a, b, count(*), max(a), max(b) FROM gstest3 GROUP BY GROUPING SETS(a, b,()) ORDER BY a, b;
|
||||||
|
COMMIT;
|
||||||
|
|
||||||
-- More rescan tests
|
-- More rescan tests
|
||||||
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;
|
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;
|
||||||
select array(select row(v.a,s1.*) from (select two,four, count(*) from onek group by cube(two,four) order by two,four) s1) from (values (1),(2)) v(a);
|
select array(select row(v.a,s1.*) from (select two,four, count(*) from onek group by cube(two,four) order by two,four) s1) from (values (1),(2)) v(a);
|
||||||
|
Loading…
Reference in New Issue
Block a user