Re: pg_createsubscriber: drop pre-existing subscriptions from the converted node

On Tue, Jul 2, 2024 at 9:57 AM Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> wrote:
>
> On Mon, 1 Jul 2024 at 11:44, Hayato Kuroda (Fujitsu)
> <kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
> >
> > Dear Amit,
> >
> > Thanks for giving comments! PSA new version.
> >
> > > Thanks, this is a better approach. I have changed a few comments and
> > > made some other cosmetic changes. See attached.
> >
> > I checked your attached and LGTM. Based on that, I added some changes
> > like below:
> >
> > - Made dbname be escaped while listing up pre-existing subscriptions
> > Previous version could not pass tests by recent commits.
> > - Skipped dropping subscriptions in dry_run mode
> > I found the issue while poring the test to 040_pg_createsubscriber.pl.
> > - Added info-level output to follow other drop_XXX functions
> >
> > > BTW, why have you created a separate test file for this test? I think
> > > we should add a new test to one of the existing tests in
> > > 040_pg_createsubscriber.
> >
> > I was separated a test file for just confirmation purpose, I've planned to merge.
> > New patch set did that.
> >
> > > You can create a dummy subscription on node_p
> > > and do a test similar to what we are doing in "# Create failover slot
> > > to test its removal".
> >
> > Your approach looks better than mine. I followed the approach.
>
> Hi Kuroda-san,
>
> I tested the patches on linux and windows and I confirm that it
> successfully fixes the issue [1].
>

Thanks for the verification. I have pushed the patch.

--
With Regards,
Amit Kapila.