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

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 and regards,
Shubham Khanna.

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

On Mon, Dec 2, 2024 at 4:57 PM vignesh C <vignesh21@gmail.com> wrote:

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:

I have fixed the suggested changes. The attached Patch contains the
required changes.

Thanks and regards,
Shubham Khanna.

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

On Tue, Dec 3, 2024 at 7:16 AM Peter Smith <smithpb2250@gmail.com> wrote:

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

I have fixed the suggested changes. The attached Patch contains the
required changes.

Thanks and regards,
Shubham Khanna.

On Tue, Dec 3, 2024 at 8:39 AM Shubham Khanna
<khannashubham1197@gmail.com> wrote:

On Tue, Dec 3, 2024 at 7:16 AM Peter Smith <smithpb2250@gmail.com> wrote:

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

I have fixed the suggested changes. The attached Patch contains the
required changes.

I have omitted the 'switch' from the suggested places to ensure
consistency within the documentation. The attached patch includes the
updated changes.

Thanks and regards,
Shubham Khanna.

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]/messages/by-id/CAHut+Ps8Xq-e5XL=AejiX-pF0417Vc7tMrh+YuZjBt7ozjvPUA@mail.gmail.com #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]: /messages/by-id/CAHut+Ps8Xq-e5XL=AejiX-pF0417Vc7tMrh+YuZjBt7ozjvPUA@mail.gmail.com
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]: /messages/by-id/CAHut+Ps8Xq-e5XL=AejiX-pF0417Vc7tMrh+YuZjBt7ozjvPUA@mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia

On Tue, Dec 3, 2024 at 9:21 AM Peter Smith <smithpb2250@gmail.com> wrote:

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] /messages/by-id/CAHut+Ps8Xq-e5XL=AejiX-pF0417Vc7tMrh+YuZjBt7ozjvPUA@mail.gmail.com

I have fixed the suggested changes. The attached Patch contains the
required changes.

Thanks and regards,
Shubham Khanna.

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

On Tue, Dec 3, 2024 at 10:25 AM Peter Smith <smithpb2250@gmail.com> wrote:

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:"

I have added the suggested change. The attached Patch contains the
required changes.

Thanks and regards,
Shubham Khanna.

Patch v6-0001 LGTM.

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

On Tue, Dec 03, 2024 at 04:58:04PM +1100, Peter Smith wrote:

Patch v6-0001 LGTM.

Right, it's true that this is a bit confusing when worded with terms
that are not in the description of the command.

The docs tend to prefer "the <option>--foo</option> option" when
referring to a specific switch in the notes section of a tool. I've
just used that, and applied the patch down to v17. Thanks!
--
Michael