diff options
| author | Jeff Davis <jdavis@postgresql.org> | 2020-12-26 17:25:30 -0800 |
|---|---|---|
| committer | Jeff Davis <jdavis@postgresql.org> | 2020-12-26 17:25:30 -0800 |
| commit | 05c0258966b502fae1bd63dcbe74d52f5c6f6948 (patch) | |
| tree | 94bb5ab4108b8a523e39ef57101c288cf8d4a673 /src/backend/executor/nodeAgg.c | |
| parent | ccbe34139b7d9059f4af3405b2411f653cc2ce7c (diff) | |
| download | postgresql-05c0258966b502fae1bd63dcbe74d52f5c6f6948.tar.gz postgresql-05c0258966b502fae1bd63dcbe74d52f5c6f6948.zip | |
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
Diffstat (limited to 'src/backend/executor/nodeAgg.c')
| -rw-r--r-- | src/backend/executor/nodeAgg.c | 24 |
1 files changed, 17 insertions, 7 deletions
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index dc640feb631..da483268cf7 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -1750,9 +1750,15 @@ hashagg_recompile_expressions(AggState *aggstate, bool minslot, bool nullcheck) const TupleTableSlotOps *outerops = aggstate->ss.ps.outerops; bool outerfixed = aggstate->ss.ps.outeropsfixed; bool dohash = true; - bool dosort; + bool dosort = false; - dosort = aggstate->aggstrategy == AGG_MIXED ? true : false; + /* + * If minslot is true, that means we are processing a spilled batch + * (inside agg_refill_hash_table()), and we must not advance the + * sorted grouping sets. + */ + if (aggstate->aggstrategy == AGG_MIXED && !minslot) + dosort = true; /* temporarily change the outerops while compiling the expression */ if (minslot) @@ -2593,11 +2599,15 @@ agg_refill_hash_table(AggState *aggstate) batch->used_bits, &aggstate->hash_mem_limit, &aggstate->hash_ngroups_limit, NULL); - /* there could be residual pergroup pointers; clear them */ - for (int setoff = 0; - setoff < aggstate->maxsets + aggstate->num_hashes; - setoff++) - aggstate->all_pergroups[setoff] = NULL; + /* + * Each batch only processes one grouping set; set the rest to NULL so + * that advance_aggregates() knows to ignore them. We don't touch + * pergroups for sorted grouping sets here, because they will be needed if + * we rescan later. The expressions for sorted grouping sets will not be + * evaluated after we recompile anyway. + */ + MemSet(aggstate->hash_pergroup, 0, + sizeof(AggStatePerGroup) * aggstate->num_hashes); /* free memory and reset hash tables */ ReScanExprContext(aggstate->hashcontext); |