Re: Possible typo in nodeAgg.c

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: "Hou, Zhijie" <houzj(dot)fnst(at)cn(dot)fujitsu(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Possible typo in nodeAgg.c
Date: 2023-11-03 01:30:07
Message-ID: CAApHDvq=01dkHwD502oUiO9wc_Ke-hKv6JncHTfkm2o0uZObjg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, 3 Nov 2023 at 13:49, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>
> On Fri, Oct 16, 2020 at 09:03:52AM +0000, Hou, Zhijie wrote:
> > /*
> > * Don't set the limit below 3/4 of hash_mem. In that case, we are at the
> > * minimum number of partitions, so we aren't going to dramatically exceed
> > - * work mem anyway.
> > + * hash_mem anyway.
>
> Can someone comment on this? Is the text change correct?

"work mem" is incorrect. I'd prefer it if we didn't talk about
hash_mem as if it were a thing. It's work_mem * hash_mem_multiplier.
Because of the underscore, using "hash_mem" to mean this makes it look
like we're talking about a variable by that name. Maybe it would be
better to refer to the variable name that's used to store the result
of get_hash_memory_limit(), i.e. hash_mem_limit. "the limit" should
likely use "*mem_limit" instead as there are multiple limits
mentioned.

It would also be better if this comment explained what's special about
4 * partition_mem. It seems to have nothing to do with the 3/4
mentioned in the comment.

David

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2023-11-03 02:03:19 Re: Tab completion regression test failed on illumos
Previous Message Thomas Munro 2023-11-03 01:22:12 Re: Tab completion regression test failed on illumos