Remove redundant assignment in CreateWorkExprContext
Hi,
While discussing [1]/messages/by-id/CAA4eK1+SEus_6vQay9TF_r4ow+E-Q7LYNLfsD78HaOsLSgppxQ@mail.gmail.com, I was reading execUtils.c, then I noticed this
redundant local variable assignment in CreateWorkExprContext(). The
attached patch fixed that.
[1]: /messages/by-id/CAA4eK1+SEus_6vQay9TF_r4ow+E-Q7LYNLfsD78HaOsLSgppxQ@mail.gmail.com
/messages/by-id/CAA4eK1+SEus_6vQay9TF_r4ow+E-Q7LYNLfsD78HaOsLSgppxQ@mail.gmail.com
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
v1-0001-Remove-redundant-assignemnt-in-CreateWorkExprCont.patchtext/plain; charset=UTF-8; name=v1-0001-Remove-redundant-assignemnt-in-CreateWorkExprCont.patchDownload
From 5b5462398117acd0203e760b7bf18c4ff616cf81 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Thu, 21 Aug 2025 11:35:18 +0800
Subject: [PATCH v1] Remove redundant assignemnt in CreateWorkExprContext
In CreateWorkExprContext(), maxBlockSize is initialized to
ALLOCSET_DEFAULT_MAXSIZE, and it then immediately reassigned,
thus the initialization is a redundant.
Author: Chao Li <lic@highgo.com>
---
src/backend/executor/execUtils.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index fdc65c2b42b..07b67d65b40 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -321,9 +321,7 @@ CreateExprContext(EState *estate)
ExprContext *
CreateWorkExprContext(EState *estate)
{
- Size maxBlockSize = ALLOCSET_DEFAULT_MAXSIZE;
-
- maxBlockSize = pg_prevpower2_size_t(work_mem * (Size) 1024 / 16);
+ Size maxBlockSize = pg_prevpower2_size_t(work_mem * (Size) 1024 / 16);
/* But no bigger than ALLOCSET_DEFAULT_MAXSIZE */
maxBlockSize = Min(maxBlockSize, ALLOCSET_DEFAULT_MAXSIZE);
--
2.39.5 (Apple Git-154)
On 8/21/25 5:47 AM, Chao Li wrote:
While discussing [1], I was reading execUtils.c, then I noticed this
redundant local variable assignment in CreateWorkExprContext(). The
attached patch fixed that.
Nice spotted but I think your patch reduces readability. How about this?
Andreas
Attachments:
v2-0001-Remove-redundant-assignemnt-in-CreateWorkExprCont.patchtext/x-patch; charset=UTF-8; name=v2-0001-Remove-redundant-assignemnt-in-CreateWorkExprCont.patchDownload
From c0f55c12069bf1b0631954cf2641d43fcdf81859 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <lic@highgo.com>
Date: Thu, 21 Aug 2025 11:35:18 +0800
Subject: [PATCH v2] Remove redundant assignemnt in CreateWorkExprContext
In CreateWorkExprContext(), maxBlockSize is initialized to
ALLOCSET_DEFAULT_MAXSIZE, and it then immediately reassigned,
thus the initialization is a redundant.
Author: Chao Li <lic@highgo.com>
---
src/backend/executor/execUtils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index cc3c5de71eb..a7955e476f9 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -321,7 +321,7 @@ CreateExprContext(EState *estate)
ExprContext *
CreateWorkExprContext(EState *estate)
{
- Size maxBlockSize = ALLOCSET_DEFAULT_MAXSIZE;
+ Size maxBlockSize;
maxBlockSize = pg_prevpower2_size_t(work_mem * (Size) 1024 / 16);
--
2.47.3
Andreas Karlsson <andreas@proxel.se> writes:
On 8/21/25 5:47 AM, Chao Li wrote:
While discussing [1], I was reading execUtils.c, then I noticed this
redundant local variable assignment in CreateWorkExprContext(). The
attached patch fixed that.
Nice spotted but I think your patch reduces readability. How about this?
Looking at the git history, CreateWorkExprContext was introduced in
50a38f651, and at that time it did some nontrivial calculations
to adjust that initial value of maxBlockSize. Later, cc721c459
simplified matters but forgot to remove the now-useless
initialization. So +1, unless Jeff has some reason to keep it
like this?
regards, tom lane
On Jan 14, 2026, at 15:13, Andreas Karlsson <andreas@proxel.se> wrote:
On 8/21/25 5:47 AM, Chao Li wrote:
While discussing [1], I was reading execUtils.c, then I noticed this redundant local variable assignment in CreateWorkExprContext(). The attached patch fixed that.
Nice spotted but I think your patch reduces readability. How about this?
Andreas
<v2-0001-Remove-redundant-assignemnt-in-CreateWorkExprCont.patch>
Hi Andraes,
Thank you very much for reviewing and bumping this patch. I’m fine with your tuning; I don’t have a strong preference, so I’m happy to leave the final decision to the committers.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
On Wed, 2026-01-14 at 02:33 -0500, Tom Lane wrote:
Looking at the git history, CreateWorkExprContext was introduced in
50a38f651, and at that time it did some nontrivial calculations
to adjust that initial value of maxBlockSize. Later, cc721c459
simplified matters but forgot to remove the now-useless
initialization. So +1, unless Jeff has some reason to keep it
like this?
Right. Committed, thank you.
Regards,
Jeff Davis
Hi,
On Wed, Jan 14, 2026 at 12:03:01PM -0800, Jeff Davis wrote:
On Wed, 2026-01-14 at 02:33 -0500, Tom Lane wrote:
Looking at the git history, CreateWorkExprContext was introduced in
50a38f651, and at that time it did some nontrivial calculations
to adjust that initial value of maxBlockSize. Later, cc721c459
simplified matters but forgot to remove the now-useless
initialization. So +1, unless Jeff has some reason to keep it
like this?Right. Committed, thank you.
FWIW, this thread gave me the idea to check for such cases in the entire code
tree. I did it with the help of [1]https://github.com/bdrouvot/coccinelle_on_pg/blob/main/misc/useless_assignment.cocci, and that would give:
"
85 files changed, 116 insertions(+), 116 deletions(-)
"
This is quite large, so if we think that's worth the time and energy we could
update a subset of files at a time per month.
That would:
- keep changes consistent within each file
- ease the review(s)
- avoid "large" rebases for patches waiting in the commitfest
Thoughts?
[1]: https://github.com/bdrouvot/coccinelle_on_pg/blob/main/misc/useless_assignment.cocci
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com