Re: Additional size of hash table is alway zero for hash aggregates

Hi,

On 2020-03-13 00:34:22 +0000, Andrew Gierth wrote:
> >>>>> "Justin" == Justin Pryzby <pryzby(at)telsasoft(dot)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