On Fri, Jul 17, 2020 at 5:13 PM Jeff Davis <pg...@j-davis.com> wrote: > The patch itself looks reasonable to me. I don't see a lot of obvious > dangers, but perhaps someone would like to take a closer look at the > planner changes as you suggest.
Attached is v3 of the hash_mem_multiplier patch series, which now has a preparatory patch that removes hashagg_avoid_disk_plan. What do you think of this approach, Jeff? It seems as if removing hashagg_avoid_disk_plan will necessitate removing various old bits of planner.c that were concerned with avoiding hash aggs that spill (the bits that hashagg_avoid_disk_plan skips in the common case where it's turned off). This makes v3-0001-* a bit trickier than I imagined it would have to be. At least it lowers the footprint of the hash_mem_multiplier code added by v3-0002-* (compared to the last version of the patch). I find the partial group paths stuff added to planner.c by commit 4f15e5d09de rather confusing (that commit was preparatory work for the main feature commit e2f1eb0e). Hopefully the hash_mem_multiplier-removal patch didn't get anything wrong in this area. Perhaps Robert can comment on this as the committer of record for partition-wise grouping/aggregation. I would like to commit this patch series by next week, and close out the two relevant open items. Separately, I suspect that we'll also need to update the cost model for hash aggs that spill, but that now seems like a totally unrelated matter. I'm waiting to hear back from Tomas about that. Tomas? Thanks -- Peter Geoghegan
v3-0001-Remove-hashagg_avoid_disk_plan-GUC.patch
Description: Binary data
v3-0002-Add-hash_mem_multiplier-GUC.patch
Description: Binary data
