On Tue, May 04, 2021 at 10:35:02PM -0400, Tom Lane wrote: > Peter Smith <smithpb2...@gmail.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@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