Обсуждение: Remove redundant assignment in CreateWorkExprContext

Hi,

While discussing [1], I was reading execUtils.c, then I noticed this 
redundant local variable assignment in CreateWorkExprContext(). The 
attached patch fixed that.

[1]: 
https://www.postgresql.org/message-id/CAA4eK1+SEus_6vQay9TF_r4ow+E-Q7LYNLfsD78HaOsLSgppxQ@mail.gmail.com

Best regards,

--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

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

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,
soI’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], 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