Re: Remove redundant assignment of a variable in function AlterPublicationTables

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, 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 23:55:23
Message-ID: CAHut+Pua62ewP+LL+Use4eML2ssi28wPoz=koJpQWmZk2KPrNQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Aug 21, 2025 at 10:11 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
...
>
> It does not matter to leave the code as is. We have a bunch of these
> depending on how people feel on the day when they implement something.
>

Hi Michael.

Some declaration assignments may be arbitrary, but I think 'isNull'
follows a well-established pattern in PG source.

FWIW, here are some search results:

Search for "&isNull[,)]" ==> ~427

Search for declarations assignments like
"bool\s*isNull\s*=\s*(true|false)" ==> ~12

Of those 12 declarations, I found only ~4 that go on to pass
unconditionally to functions by &isNull:
- AlterPublicationTables() in publicationcmds.c (this patch)
- percentile_disc_multi_final() in orderedsetaggs.c
- CatalogCacheComputeTupleHashValue() in catcache.c
- pltcl_returnnext() in pltcl.c

To summarise:
Only ~4 places are redundantly assigning isNull values before calling
functions that use &isNull.
But ~400 function calls that are passing &isNull do not pre-assignment
of that variable.

Choosing to keep this patch would be consistent with 99% of existing examples.

~~

Aside:

Of course, I recognise this is a small change that may not seem worth
the discussion overhead. However, I feel it is indicative of a problem
that's worth addressing.

During patch reviews, when I suggest fixing small issues alongside the
main changes (e.g. "let's clean up this redundant assignment while
we're here"), the typical response is that unrelated changes should be
submitted separately to keep patches focused.

But when these minor fixes are submitted as standalone patches,
they're often dismissed as too trivial to be worth the review effort
and commit overhead.

This creates a catch-22 where small but legitimate improvements never
get addressed. They fall into a gap where changes are either "too
small to include" or "too small to stand alone." This leaves valid
improvements in limbo indefinitely.

Do you have any advice for how to handle such changes?

======
Kind Regards,
Peter Smith.
Futjisu Australia

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message torikoshia 2025-08-22 00:28:49 Re: [Proposal] Expose internal MultiXact member count function for efficient monitoring
Previous Message Masahiko Sawada 2025-08-21 23:55:19 Re: memory leak in logical WAL sender with pgoutput's cachectx