Hi, On 2020-03-13 00:34:22 +0000, Andrew Gierth wrote: > >>>>> "Justin" == Justin Pryzby <pry...@telsasoft.com> writes: > > > On Thu, Mar 12, 2020 at 12:16:26PM -0700, Andres Freund wrote: > >> Indeed, that's incorrect. Causes the number of buckets for the > >> hashtable to be set higher - the size is just used for that. I'm a > >> bit wary of changing this in the stable branches - could cause > >> performance changes? > > I think (offhand, not tested) that the number of buckets would only be > affected if the (planner-supplied) numGroups value would cause work_mem > to be exceeded; the planner doesn't plan a hashagg at all in that case > unless forced to (grouping by a hashable but not sortable column). Note > that for various reasons the planner tends to over-estimate the memory > requirement anyway.
That's a good point. > Or maybe if work_mem had been reduced between plan time and execution > time.... > > So this is unlikely to be causing any issue in practice, so backpatching > may not be called for. Sounds sane to me. > I'll deal with it in HEAD only, unless someone else has a burning desire > to take it. Feel free. I wonder if we should just remove the parameter though? I'm not sure there's much point in having it, given it's just callers filling ->additionalstate. And the nbuckets is passed in externally anyway - so there needs to have been a memory sizing determination previously anyway? The other users just specify 0 already. Greetings, Andres Freund
