pg_createsubscriber: allow duplicate subscription names

Dear Chao,

Thanks for working on it. I checked your patch and briefly looks good.

```
+	if (duplicate_sub_name && num_replslots == 0)
+	{
+		pg_log_error("duplicate subscription names require distinct replication slot names");
+		pg_log_error_detail("When replication slot names are not specified, subscription names are used, but replication slot names must be unique within a cluster.");
+		pg_log_error_hint("Specify a unique --replication-slot name for each database.");
+		exit(1);
+	}
```

Not sure pg_log_error_detail() is helpful here. Can we remove?
Also I feel we do not have to clarify "distinct/unique" for replication slot names,
it has already been described other lines.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

On Aug 6, 2026, at 15:09, Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> wrote:

Dear Chao,

Thanks for working on it. I checked your patch and briefly looks good.

Thank you very much for reviewing the patch.

```
+ if (duplicate_sub_name && num_replslots == 0)
+ {
+ pg_log_error("duplicate subscription names require distinct replication slot names");
+ pg_log_error_detail("When replication slot names are not specified, subscription names are used, but replication slot names must be unique within a cluster.");
+ pg_log_error_hint("Specify a unique --replication-slot name for each database.");
+ exit(1);
+ }
```

Not sure pg_log_error_detail() is helpful here. Can we remove?

I thought to provide more info, maybe too verbose.

Also I feel we do not have to clarify "distinct/unique" for replication slot names,
it has already been described other lines.

Okay, removed.

PFA v2 - addressed Hayato-san’s comments.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

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

On Aug 7, 2026, at 08:24, Peter Smith <smithpb2250@gmail.com> wrote:

Some minor review comments for patch v2. Nothing functional.

Thank you very much for reviewing.

======
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.

Accepted.

======
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/

Accepted.

~~~

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.

Okay, why not.

~~~

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 ...
}
}

Accepted.

PFA v3: addressed Peter’s comments.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

Patch v3 LGTM

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