Horton

Thread

Commits

  1. doc: Clarify some terms for pg_createsubscriber

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

    The Post Office <noreply@postgresql.org> — 2024-11-29T12:06:20Z

    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,

  2. Re: pg_createsubscriber: publication-name and subscription-name options do not exist

    Shubham Khanna <khannashubham1197@gmail.com> — 2024-12-02T09:30:13Z

    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.

  3. Re: pg_createsubscriber: publication-name and subscription-name options do not exist

    vignesh C <vignesh21@gmail.com> — 2024-12-02T11:27:34Z

    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

  4. Re: pg_createsubscriber: publication-name and subscription-name options do not exist

    Shubham Khanna <khannashubham1197@gmail.com> — 2024-12-02T17:32:09Z

    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.

  5. Re: pg_createsubscriber: publication-name and subscription-name options do not exist

    Peter Smith <smithpb2250@gmail.com> — 2024-12-03T01:45:42Z

    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

  6. Re: pg_createsubscriber: publication-name and subscription-name options do not exist

    Shubham Khanna <khannashubham1197@gmail.com> — 2024-12-03T03:09:49Z

    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.

  7. Re: pg_createsubscriber: publication-name and subscription-name options do not exist

    Shubham Khanna <khannashubham1197@gmail.com> — 2024-12-03T03:28:16Z

    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.

  8. Re: pg_createsubscriber: publication-name and subscription-name options do not exist

    Peter Smith <smithpb2250@gmail.com> — 2024-12-03T03:50:43Z

    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

  9. Re: pg_createsubscriber: publication-name and subscription-name options do not exist

    Shubham Khanna <khannashubham1197@gmail.com> — 2024-12-03T04:22:25Z

    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] https://www.postgresql.org/message-id/CAHut%2BPs8Xq-e5XL%3DAejiX-pF0417Vc7tMrh%2BYuZjBt7ozjvPUA%40mail.gmail.com
>

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

Thanks and regards,
Shubham Khanna.

  10. Re: pg_createsubscriber: publication-name and subscription-name options do not exist

    Peter Smith <smithpb2250@gmail.com> — 2024-12-03T04:54:48Z

    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

  11. Re: pg_createsubscriber: publication-name and subscription-name options do not exist

    Shubham Khanna <khannashubham1197@gmail.com> — 2024-12-03T05:11:36Z

    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.

  12. Re: pg_createsubscriber: publication-name and subscription-name options do not exist

    Peter Smith <smithpb2250@gmail.com> — 2024-12-03T05:58:04Z

    Patch v6-0001 LGTM.

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

  13. Re: pg_createsubscriber: publication-name and subscription-name options do not exist

    Michael Paquier <michael@paquier.xyz> — 2024-12-03T07:23:31Z

    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