Re: AlterSubscription_refresh "wrconn" wrong variable?

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: AlterSubscription_refresh "wrconn" wrong variable?
Date: 2021-05-10 02:20:00
Message-ID: CAHut+Pu4Nov+9uFJiW1FM72qDB6Bm-nuMiAX_RgiXigQTncfeA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, May 7, 2021 at 6:09 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> On Fri, May 7, 2021 at 7:08 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> writes:
> > > On 2021-May-06, Peter Smith wrote:
> > >> PSA v3 of the patch. Same as before, but now also renames the global
> > >> variable from "wrconn" to "lrep_worker_wrconn".
> >
> > > I think there are two patches here -- the changes to
> > > AlterSubscription_refresh are a backpatchable bugfix, and the rest of it
> > > can just be applied to master.
> >
> > The rename of that variable is just cosmetic, true, but I'd still be
> > inclined to back-patch it. If we don't do so then I'm afraid that
> > future back-patched fixes might be bitten by the same confusion,
> > possibly introducing new real bugs.
> >
> > Having said that, keeping the two aspects in separate patches might
> > ease review and testing.
>
> Done.
>
> >
> > > In my mind we make a bit of a distinction for global variables by using
> > > CamelCase rather than undercore_separated_words.
> >
> > I think it's about 50/50, TBH. I'd stick with whichever style is
> > being used in nearby code.
> >
>
> The nearby code was a random mixture of Camels and Snakes, so instead
> of flipping a coin I went with the suggestion from Alvaro.
>
> ~~
>
> PSA v4 of the patch.
>
> 0001 - Fixes the AlterSubscription_refresh as before.
> 0002 - Renames the global var "wrconn" -> "LogRepWorkerWalRcvConn" as suggested.
>

It seems that the 0001 part of this patch was pushed in the weekend [1]. Thanks!

But what about the 0002 part? If there is no immediate plan to push
that also then I will post a v5 just to stop the cfbot complaining.

--------
[1] https://github.com/postgres/postgres/commit/4e8c0f1a0d0d095a749a329a216c88a340a455b6

KInd Regards,
Peter Smith
Fujitsu Australia

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Etsuro Fujita 2021-05-10 02:20:09 Re: Inherited UPDATE/DELETE vs async execution
Previous Message Peter Smith 2021-05-10 01:41:27 Re: Corrected documentation of data type for the logical replication message formats.