Possible typo in nodeAgg.c
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
Attachments:
0001-fix-typo-in-nodeAgg.c.patchapplication/octet-stream; name=0001-fix-typo-in-nodeAgg.c.patchDownload
From f2dc0c06824f4361f81e5d866fda4d67f6340da8 Mon Sep 17 00:00:00 2001
From: root <root@localhost.localdomain>
Date: Fri, 16 Oct 2020 04:45:06 -0400
Subject: [PATCH] fix typo in nodeAgg.c
---
src/backend/executor/nodeAgg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 75e5bbf..255d014 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -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.
*/
if (hash_mem * 1024L > 4 * partition_mem)
*mem_limit = hash_mem * 1024L - partition_mem;
--
1.8.3.1
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