Re: Enhance 'pg_createsubscriber' to retrieve databases automatically when no database is provided.

Hi Shubham.

Responding with a blanket statement "Fixed the given comments" makes
it difficult to be sure what was done when I come to confirm the
changes. Often embedded questions go unanswered, and if changes are
*not* made, then I can't tell if there was a good reason for rejection
or was the comment just overlooked. Please can you treat the itemised
feedback as a checklist and reply individually to each item. Even just
saying "Fixed" is useful because then I can trust the item was seen
and addressed.

e.g. From previous review [1]
#7. Not fixed. The same docs issue still exists for --publication and
for --subscription.
#13. Unanswered question "How are tests expecting this even passing?".
Was a reason identified? IOW, how can we be sure the latest tests
don't have a similar problem?

//////

Some more review comments for the patch v5-0001.

======
doc/src/sgml/ref/pg_createsubscriber.sgml

Synopsis

1.
pg_createsubscriber [option...] { -a | --all-databases } { -d |
--database }dbname { -D | --pgdata }datadir { -P | --publisher-server
}connstr

This looks misleading because '-all-database' and '--database' cannot
be combined.

I think it will be better to have 2 completely different synopsis like
I had originally suggested [2, comment #5]. E.g. just add a whole
seperate <cmdsynopsis> block adjacent to the other one in the SGML:

------
<cmdsynopsis>
<command>pg_createsubscriber</command>
<arg rep="repeat"><replaceable>option</replaceable></arg>
<group choice="plain">
<group choice="req">
<arg choice="plain"><option>-a</option></arg>
<arg choice="plain"><option>--all-databases</option></arg>
</group>
<group choice="req">
<arg choice="plain"><option>-D</option> </arg>
<arg choice="plain"><option>--pgdata</option></arg>
</group>
<replaceable>datadir</replaceable>
<group choice="req">
<arg choice="plain"><option>-P</option></arg>
<arg choice="plain"><option>--publisher-server</option></arg>
</group>
<replaceable>connstr</replaceable>
</group>
</cmdsynopsis>
------

~~~

2. --all-databases

+ <para>
+ <option>--all-databases</option> cannot be used with
+ <option>--database</option>, <option>--publication</option>,
+ <option>--replication-slot</option> or <option>--subscription</option>.
+ </para>

If you want consistent wording with the other places in this patch,
then this should just be the last sentence of the previous paragraph
and be worded like: "Cannot be used together with..."

~~~

3. --publication

- a generated name is assigned to the publication name.
+ a generated name is assigned to the publication name. Cannot be used
+ together with <option>-a</option>.

Previously reported. Claimed fixed -a to --all-databases. Not fixed.

~~~

4. --subscription

- a generated name is assigned to the subscription name.
+ a generated name is assigned to the subscription name. Cannot be used
+ together with <option>-a</option>.

(same as the previous review comment).

Previously reported. Claimed fixed -a to --all-databases. Not fixed.

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

5.
+ bool all_databases; /* fetch and specify all databases */

Comment wording. "and specified" (??). Maybe just "--all-databases
option was specified"

======
[1] https://www.postgresql.org/message-id/CAHut%2BPuYmTyi6kPFnJDTvD%3DaLHd0kyX4VG6iDQVEk-ixov1AwA%40mail.gmail.com
[2] https://www.postgresql.org/message-id/CAHut%2BPuyBsOJTSygus2-yp60sw_phwYQ-JyC%2BU6fCBMos9x2LA%40mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia