Thread: pgsql: Fix bug #16784 in Disk-based Hash Aggregation.
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 Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/05c0258966b502fae1bd63dcbe74d52f5c6f6948 Modified Files -------------- src/backend/executor/nodeAgg.c | 24 +++- src/test/regress/expected/groupingsets.out | 207 +++++++++++++++++++++++++++++ src/test/regress/sql/groupingsets.sql | 21 +++ 3 files changed, 245 insertions(+), 7 deletions(-)
Jeff Davis <jdavis@postgresql.org> writes:
> Fix bug #16784 in Disk-based Hash Aggregation.
Buildfarm thinks this new test case is not completely stable.
It appears that only 32-bit machines are failing, and it's
not totally consistent even for them. Dunno what to make of it.
regards, tom lane
On Sun, 2020-12-27 at 02:09 -0500, Tom Lane wrote:
> Jeff Davis <jdavis@postgresql.org> writes:
> > Fix bug #16784 in Disk-based Hash Aggregation.
>
> Buildfarm thinks this new test case is not completely stable.
>
> It appears that only 32-bit machines are failing, and it's
> not totally consistent even for them. Dunno what to make of it.
I think I just need to disable sort. Because there is a () group, we
don't need a Sort in the plan to exercise the bug.
Committed to master. I'll backport if the tests stabilize. I'm on
vacation now and will return on the 5th, so hopefully we're in a
reasonable state until then. If not, I can just revert my fix and deal
with it when I get back.
Regards,
Jeff Davis
On Sun, 2020-12-27 at 09:59 -0800, Jeff Davis wrote:
> I think I just need to disable sort. Because there is a () group, we
> don't need a Sort in the plan to exercise the bug.
That still didn't stabilize the test, so I removed the EXPLAIN part of
the test and left the execution test in place.
The planning for grouping sets still uses work_mem in the knapsack
algorithm, so something about the other platforms is not considering
the plan with all hashes (except the () grouping set). I think we can
tune the test to be stable, but it doesn't seem critical so I think
we're fine just doing the execution-time test.
Regards,
Jeff Davis