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

Commit 7b8692e

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 9f8a48b commit 7b8692e

File tree

3 files changed

+245
-7
lines changed

3 files changed

+245
-7
lines changed

‎src/backend/executor/nodeAgg.c

+17-7
Original file line numberDiff line numberDiff line change
@@ -1758,9 +1758,15 @@ hashagg_recompile_expressions(AggState *aggstate, bool minslot, bool nullcheck)
17581758
const TupleTableSlotOps *outerops = aggstate->ss.ps.outerops;
17591759
bool outerfixed = aggstate->ss.ps.outeropsfixed;
17601760
bool dohash = true;
1761-
bool dosort;
1761+
bool dosort = false;
17621762

1763-
dosort = aggstate->aggstrategy == AGG_MIXED ? true : false;
1763+
/*
1764+
* If minslot is true, that means we are processing a spilled batch
1765+
* (inside agg_refill_hash_table()), and we must not advance the
1766+
* sorted grouping sets.
1767+
*/
1768+
if (aggstate->aggstrategy == AGG_MIXED && !minslot)
1769+
dosort = true;
17641770

17651771
/* temporarily change the outerops while compiling the expression */
17661772
if (minslot)
@@ -2601,11 +2607,15 @@ agg_refill_hash_table(AggState *aggstate)
26012607
batch->used_bits, &aggstate->hash_mem_limit,
26022608
&aggstate->hash_ngroups_limit, NULL);
26032609

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

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

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

+207
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

+21
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)