From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
---|---|
To: | Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Kirill Reshke <reshkekirill(at)gmail(dot)com> |
Subject: | Re: Remove redundant assignment of a variable in function AlterPublicationTables |
Date: | 2025-08-21 01:10:24 |
Message-ID: | CAHut+Pux31GWW7T377WifAGH_ChvD1x=kiq9C7ch7Ce_cX3ptw@mail.gmail.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Aug 20, 2025 at 6:41 PM Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> wrote:
>
> Hi,
>
> While working on the thread [1]. We encountered that in the
> 'AlterPublicationTables' function, the assignment 'isnull = true' is
> redundant. This assignment is not required, and the variable will be
> reassigned before use.
> I have attached a patch to address this.
>
Since I gave this review comment in the first place, I feel obliged to say +1
I feel that having redundant assignments can be misleading because
someone reading the code might think the initial value matters or has
some significance, when it does not.
~~~
Here's another example, in the same function:
----------
PublicationRelInfo *newpubrel;
Oid newrelid;
Bitmapset *newcolumns = NULL;
----------
None of those initial values above is needed because these variables
are all immediately/unconditionally reassigned. So, why is 'newpubrel'
not assigned up-front, but 'newcolumns' is assigned? IMO this implies
that the 'newcolumns' initial value has some meaning (which it
doesn't), so it just introduces unnecessary doubts for the reader...
YMMV.
======
Kind Regards,
Peter Smith
Futjisu Australia
From | Date | Subject | |
---|---|---|---|
Next Message | Justin Pryzby | 2025-08-21 01:51:16 | Re: analyze-in-stages post upgrade questions |
Previous Message | Richard Guo | 2025-08-21 01:07:33 | Re: Reduce "Var IS [NOT] NULL" quals during constant folding |