Possible typo in nodeAgg.c
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t43097psql -h localhost -U postgresBuilt from patchset v1 (message #1), October 23, 2026 at 12:59 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t43097_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t43097_1 && git checkout t43097_1Patchset v1 (message #1) is on t43097_1
Hi
In /src/backend/executor/nodeAgg.c
I found the following comment still use work mem,
Since hash_mem has been introduced, Is it more accurate to use hash_mem here ?
@@ -1827,7 +1827,7 @@ hash_agg_set_limits(double hashentrysize, double input_groups, int used_bits,
/*
* 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.Best regards,
houzj
On Fri, Oct 16, 2020 at 09:03:52AM +0000, Hou, Zhijie wrote:
Hi
In /src/backend/executor/nodeAgg.c
I found the following comment still use work mem,
Since hash_mem has been introduced, Is it more accurate to use hash_mem here ?@@ -1827,7 +1827,7 @@ hash_agg_set_limits(double hashentrysize, double input_groups, int used_bits, /* * 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?
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
On Fri, 3 Nov 2023 at 13:49, Bruce Momjian <bruce@momjian.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