Dear Shubham,
Thanks for updating the patch. Here are comments.
01.
```
/*
* Create the subscriptions, adjust the initial location for logical
* replication and enable the subscriptions. That's the last step for logical
* replication setup. If 'drop_publications' is true, existing publications on
* the subscriber will be dropped before creating new subscriptions.
*/
static void
setup_subscriber(struct LogicalRepInfo *dbinfo, const char *consistent_lsn,
bool drop_publications)
```
Even drop_publications is false, at least one publication would be dropped. The
argument is not correct. I prefer previous name.
02.
```
/* Drop all existing publications if requested */
if (drop_publications)
{
pg_log_info("Dropping all existing publications in
database \"%s\"",
dbinfo[i].dbname);
drop_all_publications(conn, dbinfo[i].dbname);
}
/*
* Since the publication was created before the consistent LSN,
it is
* available on the subscriber when the physical replica is
promoted.
* Remove publications from the subscriber because it has no
use.
* Additionally, drop publications existed before this command
if
* requested.
*/
drop_publication(conn, &dbinfo[i]);
```
It is quite strange that drop_publication() is called even when
drop_all_publications() is called.
03.
Also, I'm not sure pg_log_info() is needed here. _drop_one_publication()
outputs dropping publications.
04.
```
/* Helper function to drop a single publication by name. */
static void
_drop_one_publication(PGconn *conn, const char *pubname, const char *dbname)
```
Functions recently added do not have prefix "_". I think it can be removed.
05.
```
pg_log_debug("Executing: %s", query->data);
pg_log_info("Dropping publication %s in database \"%s\"", pubname,
dbinfo->dbname);
```
In _drop_one_publication(), dbname is used only for the message. Can we move to
pg_log_info()
outside the function and reduce the argument?
06.
Also, please start with lower case.
07.
Also, please preserve the message as much as possible. Previously they are:
```
pg_log_info("dropping publication \"%s\" in database \"%s\"", dbinfo->pubname,
dbinfo->dbname);
pg_log_debug("command is: %s", str->data);
```
08.
I feel we must update made_publication.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
