Re: pg_createsubscriber: allow duplicate subscription names

Some minor review comments for patch v2. Nothing functional.

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

1.
is reported. The order of the multiple subscription name switches must
match the order of database switches. If this option is not specified,
a generated name is assigned to the subscription name. This
option cannot
- be used together with <option>--all</option>.
+ be used together with <option>--all</option>. The same
subscription name
+ can be used in different databases only when replication slot names are
+ specified with <option>--replication-slot</option>.
</para>

The new sentence LGTM. But my AI is pointing out that now the "-all"
sentence sits wedged between the parts describing names (order of
names, generated names, same names). It recommends moving the "--all"
sentence like below:

SUGGESTION
The subscription name to set up the logical replication. This option
cannot be used together with --all. Multiple subscriptions can be
specified by writing multiple --subscription switches. The number of
subscription names must match the number of specified databases,
otherwise an error is reported. The order of the multiple subscription
name switches must match the order of database switches. If this
option is not specified, a generated name is assigned to the
subscription name. The same subscription name can be used in
different databases only when replication slot names are specified
with --replication-slot.

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

2.
int option_index;
+ bool duplicate_sub_name = false;

Should it be plural?

/duplicate_sub_name/duplicate_sub_names/

~~~

3.
+ if (simple_string_list_member(&opt.sub_names, optarg))
+ duplicate_sub_name = true;

Having found at least one duplicate, you don't really need to keep checking.

SUGGESTION
if (!duplicate_sub_name)
duplicate_sub_name = simple_string_list_member(&opt.sub_names, optarg);

Anyway, the extra checking is cheap, so feel free to ignore this comment.

~~~

4.
if (num_replslots > 0 && num_replslots != num_dbs)
{
pg_log_error("wrong number of replication slot names specified");
pg_log_error_detail("The number of specified replication slot names
(%d) must match the number of specified database names (%d).",
num_replslots, num_dbs);
exit(1);
}
if (duplicate_sub_name && num_replslots == 0)
{
pg_log_error("duplicate subscription names require replication slot names");
pg_log_error_hint("Specify --replication-slot for each database.");
exit(1);
}

The code LGTM, but would it be tidier to avoid multiple
`num_replslots` checks by combining as a single if/else?

SUGGESTION
if (num_replslots == 0)
{
if (duplicate_sub_name)
{
pg_log_error ...
}
}
else
{
if (num_replslots != num_dbs)
{
pg_log_error ...
}
}

======
Kind Regards,
Peter Smith.
Fujitsu Australia