Thread: pg_createsubscriber: publication-name and subscription-name options do not exist

pg_createsubscriber: publication-name and subscription-name options do not exist

From
PG Doc comments form
Date:
The following documentation comment has been logged on the website:

Page: https://www.postgresql.org/docs/17/app-pgcreatesubscriber.html
Description:

The page https://www.postgresql.org/docs/17/app-pgcreatesubscriber.html 
mentions these two options:
“If publication-name option is not specified …“
“If subscription-name is not specified …“

while `pg_createsubscriber --help` returns:

      --publication=NAME          publication name
      --replication-slot=NAME     replication slot name
      --subscription=NAME         subscription name

I suppose that the options should be respectively --publication and
--subscription.

Yours,

On Mon, 2 Dec 2024 at 15:00, Shubham Khanna <khannashubham1197@gmail.com> wrote:
>
> On Mon, Dec 2, 2024 at 2:57 PM PG Doc comments form
> <noreply@postgresql.org> wrote:
> >
> > The following documentation comment has been logged on the website:
> >
> > Page: https://www.postgresql.org/docs/17/app-pgcreatesubscriber.html
> > Description:
> >
> > The page https://www.postgresql.org/docs/17/app-pgcreatesubscriber.html
> > mentions these two options:
> > “If publication-name option is not specified …“
> > “If subscription-name is not specified …“
> >
> > while `pg_createsubscriber --help` returns:
> >
> >       --publication=NAME          publication name
> >       --replication-slot=NAME     replication slot name
> >       --subscription=NAME         subscription name
> >
> > I suppose that the options should be respectively --publication and
> > --subscription.
> >
>
> I have updated the Documentation for pg_createsubscriber with the
> suggested changes. The attached Patch contains the required changes.

Thanks for the patch, one suggestion:
We can change "<option>subscription</option>" to
"<option>subscription</option> option" to keep it consistent with
publication option documentation just above a few lines which mentions
it like "If <option>publication</option> option is not specified":
       Create a subscription for each specified database on the target server.
-      If <option>subscription-name</option> is not specified, the subscription
+      If <option>subscription</option> is not specified, the subscription
       has the following name pattern:
       <quote><literal>pg_createsubscriber_%u_%x</literal></quote> (parameters:

Regards,
Vignesh



Hi Shubham,

I took a look at the patch v2-0001. Here are some review comments.

1.
Previously when this DOCS page was referring to the --publication and
--subscription it was calling those "switches". I don't know why, but
it does.

Maybe calling them "options" is ok too, but personally (for
consistency with the above) I would've stuck with the same "switches"
terminology.

1a.
/If --publication option is not specified.../If the --publication
switch is not specified.../

or maybe just omit the term entirely
/If --publication option is not specified.../If --publication is not
specified.../

~

1b.
/If --subscription option is not specified.../If the --subscription
switch is not specified.../

or maybe just omit the term entirely
/If --subscription option is not specified.../If --subscription is not
specified.../

~~~

2.
Although the patch addresses the reported problems, I think the same
problem exists for "replication-slot-name". e.g. the DOCS are saying
"If replication-slot-name is not specified..." although there is no
such thing -- it is called "--replication-slot"

So, the patch should also be fixing that one using the same consistent wording.

e.g.
/If replication-slot-name is not specified.../If the
--replication-slot switch is not specified.../
or
/If replication-slot-name is not specified.../If --replication-slot is
not specified.../

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



Hi Shubham,

Here are my comments for v4-0001.

1.
-      If <option>replication-slot-name</option> is not specified, the
+      If the <option>--replication-slot-name</option> is not specified, the

No. I already said before ([1] #2) that there is no such thing called
"--replication-slot-name".  It should be called "--replication-slot"

~~~

2.
FYI your v4 changes are not using the text that I had suggested in
[1]. E.g. When the switch/option term is not included I felt it was
better to omit the word "the". Maybe you changed that deliberately, in
which case that is your choice, but TBH I suspect this may have been
unintentional.

IMO it should look like this.

/If the <option>--publication</option> is not specified/If
<option>--publication</option> is not specified/

/If the <option>--replication-slot-name</option> is not specified/If
<option>--replication-slot</option> is not specified/

/If the <option>--subscription</option> is not specified/If
<option>--subscription</option> is not specified/

======
[1] https://www.postgresql.org/message-id/CAHut%2BPs8Xq-e5XL%3DAejiX-pF0417Vc7tMrh%2BYuZjBt7ozjvPUA%40mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia



Hi Shubham,

The patch v5-0001 looks mostly OK to me (visual inspection only).

But, I did notice one other improvement you could make in passing.

1.
-      TABLES</literal></link>.  If <option>publication-name</option> option is
-      not specified, it has the following name pattern:
+      TABLES</literal></link>.  If <option>--publication</option> is not
+      specified, it has the following name pattern:

Instead of saying "it has the following name pattern:", IMO it is better to say:
"the publication has the following name pattern:".

That would make it consistent with the similar sentences which are saying:
"... the replication slot has the following name pattern:
"... the subscription has the following name pattern:"

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



Patch v6-0001 LGTM.

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