Skip to content

Commit 05c0258

Browse files
jeff-davisjeff-davis
committed
Fix bug #16784 in Disk-based Hash Aggregation.
Before processing tuples, agg_refill_hash_table() was setting all pergroup pointers to NULL to signal to advance_aggregates() that it should not attempt to advance groups that had spilled. The problem was that it also set the pergroups for sorted grouping sets to NULL, which caused rescanning to fail. Instead, change agg_refill_hash_table() to only set the pergroups for hashed grouping sets to NULL; and when compiling the expression, pass doSort=false. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/16784-7ff169bf2c3d1588%40postgresql.org Backpatch-through: 13
1 parent ccbe341 commit 05c0258

File tree

3 files changed

+245
-7
lines changed

3 files changed

+245
-7
lines changed

‎src/backend/executor/nodeAgg.c‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1750,9 +1750,15 @@ hashagg_recompile_expressions(AggState *aggstate, bool minslot, bool nullcheck)
17501750
const TupleTableSlotOps *outerops = aggstate->ss.ps.outerops;
17511751
bool outerfixed = aggstate->ss.ps.outeropsfixed;
17521752
bool dohash = true;
1753-
bool dosort;
1753+
bool dosort = false;
17541754

1755-
dosort = aggstate->aggstrategy == AGG_MIXED ? true : false;
1755+
/*
1756+
* If minslot is true, that means we are processing a spilled batch
1757+
* (inside agg_refill_hash_table()), and we must not advance the
1758+
* sorted grouping sets.
1759+
*/
1760+
if (aggstate->aggstrategy == AGG_MIXED && !minslot)
1761+
dosort = true;
17561762

17571763
/* temporarily change the outerops while compiling the expression */
17581764
if (minslot)
@@ -2593,11 +2599,15 @@ agg_refill_hash_table(AggState *aggstate)
25932599
batch->used_bits, &aggstate->hash_mem_limit,
25942600
&aggstate->hash_ngroups_limit, NULL);
25952601

2596-
/* there could be residual pergroup pointers; clear them */
2597-
for (int setoff = 0;
2598-
setoff < aggstate->maxsets + aggstate->num_hashes;
2599-
setoff++)
2600-
aggstate->all_pergroups[setoff] = NULL;
2602+
/*
2603+
* Each batch only processes one grouping set; set the rest to NULL so
2604+
* that advance_aggregates() knows to ignore them. We don't touch
2605+
* pergroups for sorted grouping sets here, because they will be needed if
2606+
* we rescan later. The expressions for sorted grouping sets will not be
2607+
* evaluated after we recompile anyway.
2608+
*/
2609+
MemSet(aggstate->hash_pergroup, 0,
2610+
sizeof(AggStatePerGroup) * aggstate->num_hashes);
26012611

26022612
/* free memory and reset hash tables */
26032613
ReScanExprContext(aggstate->hashcontext);

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

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1665,6 +1665,213 @@ select v||'a', case when grouping(v||'a') = 1 then 1 else 0 end, count(*)
16651665
| 1 | 2
16661666
(4 rows)
16671667

1668+
-- Bug #16784
1669+
CREATE TABLE bug_16784(i INT, j INT);
1670+
ANALYZE bug_16784;
1671+
ALTER TABLE bug_16784 SET (autovacuum_enabled = 'false');
1672+
UPDATE pg_class SET reltuples = 10 WHERE relname='bug_16784';
1673+
INSERT INTO bug_16784 SELECT g/10, g FROM generate_series(1,40) g;
1674+
SET work_mem='64kB';
1675+
explain (costs off) select * from
1676+
(values (1),(2)) v(a),
1677+
lateral (select v.a, i, j, count(*) from
1678+
bug_16784 group by cube(i,j)) s
1679+
order by v.a, i, j;
1680+
QUERY PLAN
1681+
----------------------------------------------------------
1682+
Sort
1683+
Sort Key: "*VALUES*".column1, bug_16784.i, bug_16784.j
1684+
-> Nested Loop
1685+
-> Values Scan on "*VALUES*"
1686+
-> MixedAggregate
1687+
Hash Key: bug_16784.i, bug_16784.j
1688+
Hash Key: bug_16784.i
1689+
Hash Key: bug_16784.j
1690+
Group Key: ()
1691+
-> Seq Scan on bug_16784
1692+
(10 rows)
1693+
1694+
select * from
1695+
(values (1),(2)) v(a),
1696+
lateral (select a, i, j, count(*) from
1697+
bug_16784 group by cube(i,j)) s
1698+
order by v.a, i, j;
1699+
a | a | i | j | count
1700+
---+---+---+----+-------
1701+
1 | 1 | 0 | 1 | 1
1702+
1 | 1 | 0 | 2 | 1
1703+
1 | 1 | 0 | 3 | 1
1704+
1 | 1 | 0 | 4 | 1
1705+
1 | 1 | 0 | 5 | 1
1706+
1 | 1 | 0 | 6 | 1
1707+
1 | 1 | 0 | 7 | 1
1708+
1 | 1 | 0 | 8 | 1
1709+
1 | 1 | 0 | 9 | 1
1710+
1 | 1 | 0 | | 9
1711+
1 | 1 | 1 | 10 | 1
1712+
1 | 1 | 1 | 11 | 1
1713+
1 | 1 | 1 | 12 | 1
1714+
1 | 1 | 1 | 13 | 1
1715+
1 | 1 | 1 | 14 | 1
1716+
1 | 1 | 1 | 15 | 1
1717+
1 | 1 | 1 | 16 | 1
1718+
1 | 1 | 1 | 17 | 1
1719+
1 | 1 | 1 | 18 | 1
1720+
1 | 1 | 1 | 19 | 1
1721+
1 | 1 | 1 | | 10
1722+
1 | 1 | 2 | 20 | 1
1723+
1 | 1 | 2 | 21 | 1
1724+
1 | 1 | 2 | 22 | 1
1725+
1 | 1 | 2 | 23 | 1
1726+
1 | 1 | 2 | 24 | 1
1727+
1 | 1 | 2 | 25 | 1
1728+
1 | 1 | 2 | 26 | 1
1729+
1 | 1 | 2 | 27 | 1
1730+
1 | 1 | 2 | 28 | 1
1731+
1 | 1 | 2 | 29 | 1
1732+
1 | 1 | 2 | | 10
1733+
1 | 1 | 3 | 30 | 1
1734+
1 | 1 | 3 | 31 | 1
1735+
1 | 1 | 3 | 32 | 1
1736+
1 | 1 | 3 | 33 | 1
1737+
1 | 1 | 3 | 34 | 1
1738+
1 | 1 | 3 | 35 | 1
1739+
1 | 1 | 3 | 36 | 1
1740+
1 | 1 | 3 | 37 | 1
1741+
1 | 1 | 3 | 38 | 1
1742+
1 | 1 | 3 | 39 | 1
1743+
1 | 1 | 3 | | 10
1744+
1 | 1 | 4 | 40 | 1
1745+
1 | 1 | 4 | | 1
1746+
1 | 1 | | 1 | 1
1747+
1 | 1 | | 2 | 1
1748+
1 | 1 | | 3 | 1
1749+
1 | 1 | | 4 | 1
1750+
1 | 1 | | 5 | 1
1751+
1 | 1 | | 6 | 1
1752+
1 | 1 | | 7 | 1
1753+
1 | 1 | | 8 | 1
1754+
1 | 1 | | 9 | 1
1755+
1 | 1 | | 10 | 1
1756+
1 | 1 | | 11 | 1
1757+
1 | 1 | | 12 | 1
1758+
1 | 1 | | 13 | 1
1759+
1 | 1 | | 14 | 1
1760+
1 | 1 | | 15 | 1
1761+
1 | 1 | | 16 | 1
1762+
1 | 1 | | 17 | 1
1763+
1 | 1 | | 18 | 1
1764+
1 | 1 | | 19 | 1
1765+
1 | 1 | | 20 | 1
1766+
1 | 1 | | 21 | 1
1767+
1 | 1 | | 22 | 1
1768+
1 | 1 | | 23 | 1
1769+
1 | 1 | | 24 | 1
1770+
1 | 1 | | 25 | 1
1771+
1 | 1 | | 26 | 1
1772+
1 | 1 | | 27 | 1
1773+
1 | 1 | | 28 | 1
1774+
1 | 1 | | 29 | 1
1775+
1 | 1 | | 30 | 1
1776+
1 | 1 | | 31 | 1
1777+
1 | 1 | | 32 | 1
1778+
1 | 1 | | 33 | 1
1779+
1 | 1 | | 34 | 1
1780+
1 | 1 | | 35 | 1
1781+
1 | 1 | | 36 | 1
1782+
1 | 1 | | 37 | 1
1783+
1 | 1 | | 38 | 1
1784+
1 | 1 | | 39 | 1
1785+
1 | 1 | | 40 | 1
1786+
1 | 1 | | | 40
1787+
2 | 2 | 0 | 1 | 1
1788+
2 | 2 | 0 | 2 | 1
1789+
2 | 2 | 0 | 3 | 1
1790+
2 | 2 | 0 | 4 | 1
1791+
2 | 2 | 0 | 5 | 1
1792+
2 | 2 | 0 | 6 | 1
1793+
2 | 2 | 0 | 7 | 1
1794+
2 | 2 | 0 | 8 | 1
1795+
2 | 2 | 0 | 9 | 1
1796+
2 | 2 | 0 | | 9
1797+
2 | 2 | 1 | 10 | 1
1798+
2 | 2 | 1 | 11 | 1
1799+
2 | 2 | 1 | 12 | 1
1800+
2 | 2 | 1 | 13 | 1
1801+
2 | 2 | 1 | 14 | 1
1802+
2 | 2 | 1 | 15 | 1
1803+
2 | 2 | 1 | 16 | 1
1804+
2 | 2 | 1 | 17 | 1
1805+
2 | 2 | 1 | 18 | 1
1806+
2 | 2 | 1 | 19 | 1
1807+
2 | 2 | 1 | | 10
1808+
2 | 2 | 2 | 20 | 1
1809+
2 | 2 | 2 | 21 | 1
1810+
2 | 2 | 2 | 22 | 1
1811+
2 | 2 | 2 | 23 | 1
1812+
2 | 2 | 2 | 24 | 1
1813+
2 | 2 | 2 | 25 | 1
1814+
2 | 2 | 2 | 26 | 1
1815+
2 | 2 | 2 | 27 | 1
1816+
2 | 2 | 2 | 28 | 1
1817+
2 | 2 | 2 | 29 | 1
1818+
2 | 2 | 2 | | 10
1819+
2 | 2 | 3 | 30 | 1
1820+
2 | 2 | 3 | 31 | 1
1821+
2 | 2 | 3 | 32 | 1
1822+
2 | 2 | 3 | 33 | 1
1823+
2 | 2 | 3 | 34 | 1
1824+
2 | 2 | 3 | 35 | 1
1825+
2 | 2 | 3 | 36 | 1
1826+
2 | 2 | 3 | 37 | 1
1827+
2 | 2 | 3 | 38 | 1
1828+
2 | 2 | 3 | 39 | 1
1829+
2 | 2 | 3 | | 10
1830+
2 | 2 | 4 | 40 | 1
1831+
2 | 2 | 4 | | 1
1832+
2 | 2 | | 1 | 1
1833+
2 | 2 | | 2 | 1
1834+
2 | 2 | | 3 | 1
1835+
2 | 2 | | 4 | 1
1836+
2 | 2 | | 5 | 1
1837+
2 | 2 | | 6 | 1
1838+
2 | 2 | | 7 | 1
1839+
2 | 2 | | 8 | 1
1840+
2 | 2 | | 9 | 1
1841+
2 | 2 | | 10 | 1
1842+
2 | 2 | | 11 | 1
1843+
2 | 2 | | 12 | 1
1844+
2 | 2 | | 13 | 1
1845+
2 | 2 | | 14 | 1
1846+
2 | 2 | | 15 | 1
1847+
2 | 2 | | 16 | 1
1848+
2 | 2 | | 17 | 1
1849+
2 | 2 | | 18 | 1
1850+
2 | 2 | | 19 | 1
1851+
2 | 2 | | 20 | 1
1852+
2 | 2 | | 21 | 1
1853+
2 | 2 | | 22 | 1
1854+
2 | 2 | | 23 | 1
1855+
2 | 2 | | 24 | 1
1856+
2 | 2 | | 25 | 1
1857+
2 | 2 | | 26 | 1
1858+
2 | 2 | | 27 | 1
1859+
2 | 2 | | 28 | 1
1860+
2 | 2 | | 29 | 1
1861+
2 | 2 | | 30 | 1
1862+
2 | 2 | | 31 | 1
1863+
2 | 2 | | 32 | 1
1864+
2 | 2 | | 33 | 1
1865+
2 | 2 | | 34 | 1
1866+
2 | 2 | | 35 | 1
1867+
2 | 2 | | 36 | 1
1868+
2 | 2 | | 37 | 1
1869+
2 | 2 | | 38 | 1
1870+
2 | 2 | | 39 | 1
1871+
2 | 2 | | 40 | 1
1872+
2 | 2 | | | 40
1873+
(172 rows)
1874+
16681875
--
16691876
-- Compare results between plans using sorting and plans using hash
16701877
-- aggregation. Force spilling in both cases by setting work_mem low

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,27 @@ select v||'a', case when grouping(v||'a') = 1 then 1 else 0 end, count(*)
457457
from unnest(array[1,1], array['a','b']) u(i,v)
458458
group by rollup(i, v||'a') order by 1,3;
459459

460+
-- Bug #16784
461+
CREATE TABLE bug_16784(i INT, j INT);
462+
ANALYZE bug_16784;
463+
ALTER TABLE bug_16784 SET (autovacuum_enabled = 'false');
464+
UPDATE pg_class SET reltuples = 10 WHERE relname='bug_16784';
465+
466+
INSERT INTO bug_16784 SELECT g/10, g FROM generate_series(1,40) g;
467+
468+
SET work_mem='64kB';
469+
470+
explain (costs off) select * from
471+
(values (1),(2)) v(a),
472+
lateral (select v.a, i, j, count(*) from
473+
bug_16784 group by cube(i,j)) s
474+
order by v.a, i, j;
475+
select * from
476+
(values (1),(2)) v(a),
477+
lateral (select a, i, j, count(*) from
478+
bug_16784 group by cube(i,j)) s
479+
order by v.a, i, j;
480+
460481
--
461482
-- Compare results between plans using sorting and plans using hash
462483
-- aggregation. Force spilling in both cases by setting work_mem low

0 commit comments

Comments
 (0)