On Tue, Jul 2, 2024 at 9:57 AM Shlok Kyal <shlok.kyal....@gmail.com> wrote: > > On Mon, 1 Jul 2024 at 11:44, Hayato Kuroda (Fujitsu) > <kuroda.hay...@fujitsu.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.