On Wed, May 5, 2021 at 8:05 AM Tom Lane <t...@sss.pgh.pa.us> 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).
I don't think "tablesync_wrconn" is the right name, because wrconn is also being used in logical replication apply worker. So something like "apply_worker_wrconn" would be more meaningful. With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com
