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

Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>

From: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
To: 'Amit Kapila' <amit.kapila16@gmail.com>
Cc: vignesh C <vignesh21@gmail.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, Euler Taveira <euler@eulerto.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Date: 2024-07-01T06:14:30Z
Lists: pgsql-hackers

Attachments

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.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/ 

Commits

  1. Drop pre-existing subscriptions from the converted subscriber.

  2. Remove unused structure member in pg_createsubscriber.c.