Re: Add support for specifying tables in pg_createsubscriber.

Hi Shubham,

Here are some v13 review comments.

======
src/bin/pg_basebackup/pg_createsubscriber.c

1.
- /*
- * In dry-run mode, we don't create publications, but we still try to drop
- * those to provide necessary information to the user.
- */
  if (!drop_all_pubs || dry_run)
- drop_publication(conn, dbinfo->pubname, dbinfo->dbname,
- &dbinfo->made_publication);
+ {
+ /*
+ * Since the publications were created before the consistent LSN, they
+ * remain on the subscriber even after the physical replica is
+ * promoted. Only drop publications that were created by
+ * pg_createsubscriber during this operation. Pre-existing
+ * publications are preserved.
+ */
+ if (!drop_all_pubs && dbinfo->made_publication)
+ drop_publication(conn, dbinfo->pubname, dbinfo->dbname,
+ &dbinfo->made_publication);
+ else if (!drop_all_pubs)
+ pg_log_info("preserve existing publication \"%s\" in database \"%s\"",
+ dbinfo->pubname, dbinfo->dbname);
+ }

1a.
Sorry, that code looks wrong to me. Notice, the result of that patch
fragment is essentially:

if (!drop_all_pubs || dry_run)
{
  if (!drop_all_pubs && dbinfo->made_publication)
    drop_publication(...);
  else if (!drop_all_pubs)
    pg_log_info("preserve existing...";
}

You've coded *every* condition say !drop_all_pubs, which is basically
saying when drop_all_pubs == true then dry_run does nothing at all
(??). Surely, that's not the intention.

Let's see a specific test case: --publication=mynew
--clean=publications --dry-run

According to your matrix, AFAICT, you expect this to log:
- "create publication mynew"
- "dropping all existing"
- "dropping other existing..." (loop)
- "drop publication mynew"

But, I don't see how that "drop publication mynew" is possible with
this code. Can you provide some test/script output as proof that it
actually works like you say it does?

~~~

1b.
The original code at least had a comment saying what it was trying to do:

- /*
- * In dry-run mode, we don't create publications, but we still try to drop
- * those to provide necessary information to the user.
- */

Why was that comment removed? In my v12 review, I suggested some
possible better wording [1] for that. Please imagine someone reading
this code without handy access to that "expected logging" matrix, and
write explanatory comments accordingly.

======
[1] https://www.postgresql.org/message-id/CAHut%2BPtk%3Ds9VweQWXatEZ7i9GiFxZn_3A5wMSE_gDO9h7jEcRA%40mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia.