Re: AlterSubscription_refresh "wrconn" wrong variable?

On Tue, May 04, 2021 at 10:35:02PM -0400, Tom Lane wrote:
> Peter Smith <smithpb2250(at)gmail(dot)com> writes:
> > This patch replaces the global "wrconn" in AlterSubscription_refresh with a local variable of the same name, making it consistent with other functions in subscriptioncmds.c (e.g. DropSubscription).
> > The global wrconn is only meant to be used for logical apply/tablesync worker.
> > Using the global/incorrect wrconn in AlterSubscription_refresh doesn't normally cause any problems, but harm is still posslble if the apply worker ever manages to do a subscription refresh. e.g. see [1]
>
> Hm. I would actually place the blame for this on whoever thought
> it was okay to name a global variable something as generic as
> "wrconn". Let's rename that while we're at it, say to something
> like "tablesync_wrconn" (feel free to bikeshed).

Yea, I think global vars should have at least 1) an underscore, or 2) a
capital, and in any case be 3) longer than 6 chars.

There's very few which violate both "arms" of that rule - should anything else
be renamed, too ?

$ git grep -E '^static [^(=]*\<[[:lower:]]{,6}(;$| =)' src/backend/'*.c'
src/backend/access/heap/vacuumlazy.c:static int elevel = -1;
src/backend/access/transam/xloginsert.c:static XLogRecData *rdatas;
src/backend/bootstrap/bootstrap.c:static MemoryContext nogc = NULL; /* special no-gc mem context */
src/backend/libpq/be-fsstubs.c:static MemoryContext fscxt = NULL;
src/backend/replication/walreceiver.c:static WalReceiverConn *wrconn = NULL;
src/backend/replication/walsender.c:static StringInfoData tmpbuf;
src/backend/storage/file/fd.c:static int nfile = 0;
src/backend/utils/misc/sampling.c:static ReservoirStateData oldrs;

pryzbyj(at)pryzbyj:~/src/postgres$ git grep -lE '^static [^(=]*\<[[:lower:]]{,6}(;$| =)' src/backend/'*.c' |xargs wc -l |sort -nr
4326 src/backend/access/heap/vacuumlazy.c
3781 src/backend/storage/file/fd.c
3698 src/backend/replication/walsender.c
1428 src/backend/replication/walreceiver.c
1227 src/backend/access/transam/xloginsert.c
1155 src/backend/bootstrap/bootstrap.c
864 src/backend/libpq/be-fsstubs.c
296 src/backend/utils/misc/sampling.c

--
Justin