Horton

Thread

Commits

  1. Doc: Make link names consistent in logical replication commands.

  2. Doc: Add more links in logical replication pages.

  1. PGDOCS - add more links in the pub/sub reference pages

    Peter Smith <smithpb2250@gmail.com> — 2023-10-06T06:44:32Z

    The "Description" and "Notes" parts of the following logical
replication PUB/SUB reference pages (almost always) link to each other
whenever a sibling command gets mentioned.

CREATE PUBLICATION
ALTER PUBLICATION
DROP PUBLICATION

CREATE SUBSCRIPTION
ALTER SUBSCRIPTION
DROP SUBSCRIPTION

~

AFAICT the only omissions are:

ALTER PUBLICATION page -- mentions ALTER SUBSCRIPTION but there is no link
DROP SUBSCRIPTION page -- mentions ALTER SUBSCRIPTION but there is no link

~

Here is a patch to add the 2 missing references:

ref/alter_subscription.sgml ==> added more ids
ref/alter_publication.sgml ==> added link to
"sql-altersubscription-refresh-publication"
ref/drop_subscription.sgml ==> added link to "sql-altersubscription"

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

  2. Re: PGDOCS - add more links in the pub/sub reference pages

    Amit Kapila <amit.kapila16@gmail.com> — 2023-10-09T04:32:11Z

    On Fri, Oct 6, 2023 at 12:15 PM Peter Smith <smithpb2250@gmail.com> wrote:
>
> Here is a patch to add the 2 missing references:
>
> ref/alter_subscription.sgml ==> added more ids
> ref/alter_publication.sgml ==> added link to
> "sql-altersubscription-refresh-publication"
> ref/drop_subscription.sgml ==> added link to "sql-altersubscription"
>

-   <varlistentry>
+   <varlistentry id="sql-altersubscription-new-owner">
     <term><replaceable class="parameter">new_owner</replaceable></term>
     <listitem>
      <para>
@@ -281,7 +281,7 @@ ALTER SUBSCRIPTION <replaceable
class="parameter">name</replaceable> RENAME TO <
     </listitem>
    </varlistentry>

-   <varlistentry>
+   <varlistentry id="sql-altersubscription-new-name">
     <term><replaceable class="parameter">new_name</replaceable></term>
     <listitem>

Shall we append 'params' in the above and other id's in the patch. For
example, sql-altersubscription-params-new-name. The other places like
alter_role.sgml and alter_table.sgml uses similar id's. Is there a
reason to be different here?

-- 
With Regards,
Amit Kapila.

  3. Re: PGDOCS - add more links in the pub/sub reference pages

    Peter Smith <smithpb2250@gmail.com> — 2023-10-09T06:45:31Z

    On Mon, Oct 9, 2023 at 3:32 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Fri, Oct 6, 2023 at 12:15 PM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > Here is a patch to add the 2 missing references:
> >
> > ref/alter_subscription.sgml ==> added more ids
> > ref/alter_publication.sgml ==> added link to
> > "sql-altersubscription-refresh-publication"
> > ref/drop_subscription.sgml ==> added link to "sql-altersubscription"
> >
>
> -   <varlistentry>
> +   <varlistentry id="sql-altersubscription-new-owner">
>      <term><replaceable class="parameter">new_owner</replaceable></term>
>      <listitem>
>       <para>
> @@ -281,7 +281,7 @@ ALTER SUBSCRIPTION <replaceable
> class="parameter">name</replaceable> RENAME TO <
>      </listitem>
>     </varlistentry>
>
> -   <varlistentry>
> +   <varlistentry id="sql-altersubscription-new-name">
>      <term><replaceable class="parameter">new_name</replaceable></term>
>      <listitem>
>

Thanks for looking at this patch!

> Shall we append 'params' in the above and other id's in the patch. For
> example, sql-altersubscription-params-new-name. The other places like
> alter_role.sgml and alter_table.sgml uses similar id's. Is there a
> reason to be different here?

In v1, I used the same pattern as on the CREATE SUBSCRIPTION page,
which doesn't look like those...

~~~

The "Parameters" section describes some things that really are parameters:

e.g.
"sql-altersubscription-name"
"sql-altersubscription-new-owner"
"sql-altersubscription-new-name">

I agree, emphasising that those ones are parameters is better. Changed
like this in v2.

"sql-altersubscription-params-name"
"sql-altersubscription-params-new-owner"
"sql-altersubscription-params-new-name">

~

But, the "Parameters" section also describes other SQL syntax clauses
which are not really parameters at all.

e.g.
"sql-altersubscription-refresh-publication"
"sql-altersubscription-enable"
"sql-altersubscription-disable"

So I felt those ones are more intuitive left as they are  -- e.g.,
instead of having ids/linkends like:

"sql-altersubscription-params-refresh-publication"
"sql-altersubscription-params-enable"
"sql-altersubscription-params-disable"

~~

PSA v2.

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

  4. Re: PGDOCS - add more links in the pub/sub reference pages

    vignesh C <vignesh21@gmail.com> — 2023-10-10T00:46:15Z

    On Mon, 9 Oct 2023 at 12:18, Peter Smith <smithpb2250@gmail.com> wrote:
>
> On Mon, Oct 9, 2023 at 3:32 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Fri, Oct 6, 2023 at 12:15 PM Peter Smith <smithpb2250@gmail.com> wrote:
> > >
> > > Here is a patch to add the 2 missing references:
> > >
> > > ref/alter_subscription.sgml ==> added more ids
> > > ref/alter_publication.sgml ==> added link to
> > > "sql-altersubscription-refresh-publication"
> > > ref/drop_subscription.sgml ==> added link to "sql-altersubscription"
> > >
> >
> > -   <varlistentry>
> > +   <varlistentry id="sql-altersubscription-new-owner">
> >      <term><replaceable class="parameter">new_owner</replaceable></term>
> >      <listitem>
> >       <para>
> > @@ -281,7 +281,7 @@ ALTER SUBSCRIPTION <replaceable
> > class="parameter">name</replaceable> RENAME TO <
> >      </listitem>
> >     </varlistentry>
> >
> > -   <varlistentry>
> > +   <varlistentry id="sql-altersubscription-new-name">
> >      <term><replaceable class="parameter">new_name</replaceable></term>
> >      <listitem>
> >
>
> Thanks for looking at this patch!
>
> > Shall we append 'params' in the above and other id's in the patch. For
> > example, sql-altersubscription-params-new-name. The other places like
> > alter_role.sgml and alter_table.sgml uses similar id's. Is there a
> > reason to be different here?
>
> In v1, I used the same pattern as on the CREATE SUBSCRIPTION page,
> which doesn't look like those...
>
> ~~~
>
> The "Parameters" section describes some things that really are parameters:
>
> e.g.
> "sql-altersubscription-name"
> "sql-altersubscription-new-owner"
> "sql-altersubscription-new-name">
>
> I agree, emphasising that those ones are parameters is better. Changed
> like this in v2.
>
> "sql-altersubscription-params-name"
> "sql-altersubscription-params-new-owner"
> "sql-altersubscription-params-new-name">
>
> ~
>
> But, the "Parameters" section also describes other SQL syntax clauses
> which are not really parameters at all.
>
> e.g.
> "sql-altersubscription-refresh-publication"
> "sql-altersubscription-enable"
> "sql-altersubscription-disable"
>
> So I felt those ones are more intuitive left as they are  -- e.g.,
> instead of having ids/linkends like:
>
> "sql-altersubscription-params-refresh-publication"
> "sql-altersubscription-params-enable"
> "sql-altersubscription-params-disable"
>
> ~~
>
> PSA v2.

I noticed a couple of other places where a link to "ALTER SUBSCRIPTION
... DISABLE" and "ALTER SUBSCRIPTION ... SET" can be specified in
drop_subscription:
To proceed in this situation, first disable the subscription by
executing  <literal>ALTER SUBSCRIPTION ... DISABLE</literal>, and then
disassociate it from the replication slot by executing <literal>ALTER
SUBSCRIPTION ... SET (slot_name = NONE)</literal>.

Regards,
Vignesh

  5. Re: PGDOCS - add more links in the pub/sub reference pages

    Peter Smith <smithpb2250@gmail.com> — 2023-10-10T03:17:25Z

    On Tue, Oct 10, 2023 at 11:46 AM vignesh C <vignesh21@gmail.com> wrote:
>
> I noticed a couple of other places where a link to "ALTER SUBSCRIPTION
> ... DISABLE" and "ALTER SUBSCRIPTION ... SET" can be specified in
> drop_subscription:
> To proceed in this situation, first disable the subscription by
> executing  <literal>ALTER SUBSCRIPTION ... DISABLE</literal>, and then
> disassociate it from the replication slot by executing <literal>ALTER
> SUBSCRIPTION ... SET (slot_name = NONE)</literal>.
>

Hi Vignesh. Thanks for the review comments.

Modified as suggested.

PSA v3.

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

  6. Re: PGDOCS - add more links in the pub/sub reference pages

    vignesh C <vignesh21@gmail.com> — 2023-10-10T06:10:46Z

    On Tue, 10 Oct 2023 at 08:47, Peter Smith <smithpb2250@gmail.com> wrote:
>
> On Tue, Oct 10, 2023 at 11:46 AM vignesh C <vignesh21@gmail.com> wrote:
> >
> > I noticed a couple of other places where a link to "ALTER SUBSCRIPTION
> > ... DISABLE" and "ALTER SUBSCRIPTION ... SET" can be specified in
> > drop_subscription:
> > To proceed in this situation, first disable the subscription by
> > executing  <literal>ALTER SUBSCRIPTION ... DISABLE</literal>, and then
> > disassociate it from the replication slot by executing <literal>ALTER
> > SUBSCRIPTION ... SET (slot_name = NONE)</literal>.
> >
>
> Hi Vignesh. Thanks for the review comments.
>
> Modified as suggested.
>
> PSA v3.

Few more instances in other logical replication related pages:
1) Another instance was in alter_subscription.sgml:
      Fetch missing table information from publisher.  This will start
      replication of tables that were added to the subscribed-to publications
      since <command>CREATE SUBSCRIPTION</command> or
      the last invocation of <command>REFRESH PUBLICATION</command>.
2) Few more instances were in logical-replication-subscription.html
2.a)    Normally, the remote replication slot is created automatically when the
    subscription is created using <command>CREATE SUBSCRIPTION</command> and it
    is dropped automatically when the subscription is dropped using
    <command>DROP SUBSCRIPTION</command>.  In some situations, however, it can
    be useful or necessary to manipulate the subscription and the underlying
    replication slot separately.
2.b)  When dropping a subscription, the remote host is not reachable.  In
       that case, disassociate the slot from the subscription
       using <command>ALTER SUBSCRIPTION</command> before attempting to drop
       the subscription.
2.c)      If a subscription is affected by this problem, the only way to resume
     replication is to adjust one of the column lists on the publication
     side so that they all match; and then either recreate the subscription,
     or use <literal>ALTER SUBSCRIPTION ... DROP PUBLICATION</literal> to
     remove one of the offending publications and add it again.
2.d) The
   transaction that produced the conflict can be skipped by using
   <command>ALTER SUBSCRIPTION ... SKIP</command> with the finish LSN
   (i.e., LSN 0/14C0378).
2.e)    Before using this function, the subscription needs to be
disabled temporarily
   either by <command>ALTER SUBSCRIPTION ... DISABLE</command> or,

Regards,
Vignesh

  7. Re: PGDOCS - add more links in the pub/sub reference pages

    Amit Kapila <amit.kapila16@gmail.com> — 2023-10-10T12:33:05Z

    On Tue, Oct 10, 2023 at 11:40 AM vignesh C <vignesh21@gmail.com> wrote:
>
> On Tue, 10 Oct 2023 at 08:47, Peter Smith <smithpb2250@gmail.com> wrote:
> > PSA v3.
>
> Few more instances in other logical replication related pages:
> 1) Another instance was in alter_subscription.sgml:
>       Fetch missing table information from publisher.  This will start
>       replication of tables that were added to the subscribed-to publications
>       since <command>CREATE SUBSCRIPTION</command> or
>       the last invocation of <command>REFRESH PUBLICATION</command>.
>

Do we want each and every occurrence of the commands to have
corresponding links? I am not against it if we think that is useful
for users but asking as I am not aware of the general practice we
follow in this regard. Does anyone else have any opinion on this
matter?

-- 
With Regards,
Amit Kapila.

  8. Re: PGDOCS - add more links in the pub/sub reference pages

    Peter Smith <smithpb2250@gmail.com> — 2023-10-11T00:28:16Z

    On Tue, Oct 10, 2023 at 11:33 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Tue, Oct 10, 2023 at 11:40 AM vignesh C <vignesh21@gmail.com> wrote:
> >
> > On Tue, 10 Oct 2023 at 08:47, Peter Smith <smithpb2250@gmail.com> wrote:
> > > PSA v3.
> >
> > Few more instances in other logical replication related pages:
> > 1) Another instance was in alter_subscription.sgml:
> >       Fetch missing table information from publisher.  This will start
> >       replication of tables that were added to the subscribed-to publications
> >       since <command>CREATE SUBSCRIPTION</command> or
> >       the last invocation of <command>REFRESH PUBLICATION</command>.
> >
>
> Do we want each and every occurrence of the commands to have
> corresponding links? I am not against it if we think that is useful
> for users but asking as I am not aware of the general practice we
> follow in this regard. Does anyone else have any opinion on this
> matter?
>

The goal of the patch was to use a consistent approach for all the
pub/sub pages. Otherwise, there was a mixture and no apparent reason
why some commands had links while some did not.

The rules this patch is using are:
- only including inter-page links to other pub/sub commands
- if the same pub/sub linkend occurs multiple times in the same block
of text, then only give a link for the first one

~~

What links are "useful to users" is subjective, and the convenience
probably also varies depending on how much scrolling is needed to get
to the "See Also" part at the bottom. I felt a consistent linking
approach is better than having differences based on some arbitrary
judgement of usefulness.

AFAICT some other PG DOCS pages strive to do the same. For example,
the ALTER TABLE page [1] mentions the "CREATE TABLE" command 10 times
and 8 of those have links. (the missing ones don't look any different
to me so seem like accidental omissions).

======
[1] https://www.postgresql.org/docs/devel/sql-altertable.html

Kind Regards,
Peter Smith.
Fujitsu Australia

  9. Re: PGDOCS - add more links in the pub/sub reference pages

    Peter Smith <smithpb2250@gmail.com> — 2023-10-11T00:31:02Z

    On Tue, Oct 10, 2023 at 5:10 PM vignesh C <vignesh21@gmail.com> wrote:
>
> Few more instances in other logical replication related pages:
> 1) Another instance was in alter_subscription.sgml:
>       Fetch missing table information from publisher.  This will start
>       replication of tables that were added to the subscribed-to publications
>       since <command>CREATE SUBSCRIPTION</command> or
>       the last invocation of <command>REFRESH PUBLICATION</command>.

OK, modified in v4.

> 2) Few more instances were in logical-replication-subscription.html
> 2.a)    Normally, the remote replication slot is created automatically when the
>     subscription is created using <command>CREATE SUBSCRIPTION</command> and it
>     is dropped automatically when the subscription is dropped using
>     <command>DROP SUBSCRIPTION</command>.  In some situations, however, it can
>     be useful or necessary to manipulate the subscription and the underlying
>     replication slot separately.
> 2.b)  When dropping a subscription, the remote host is not reachable.  In
>        that case, disassociate the slot from the subscription
>        using <command>ALTER SUBSCRIPTION</command> before attempting to drop
>        the subscription.

The above were in section "31.2.1 Replication Slot Management". OK,
modified in v4.

> 2.c)      If a subscription is affected by this problem, the only way to resume
>      replication is to adjust one of the column lists on the publication
>      side so that they all match; and then either recreate the subscription,
>      or use <literal>ALTER SUBSCRIPTION ... DROP PUBLICATION</literal> to
>      remove one of the offending publications and add it again.

The above was in section "31.2.1 Replication Slot Management". OK,
modified in v4.

> 2.d) The
>    transaction that produced the conflict can be skipped by using
>    <command>ALTER SUBSCRIPTION ... SKIP</command> with the finish LSN
>    (i.e., LSN 0/14C0378).
> 2.e)    Before using this function, the subscription needs to be
> disabled temporarily
>    either by <command>ALTER SUBSCRIPTION ... DISABLE</command> or,
>

The above was in the section "31.5 Conflicts". OK, modified in v4.

~~

Thanks for reporting those. PSA v4.

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

  10. Re: PGDOCS - add more links in the pub/sub reference pages

    Amit Kapila <amit.kapila16@gmail.com> — 2023-10-12T04:44:35Z

    On Mon, Oct 9, 2023 at 12:15 PM Peter Smith <smithpb2250@gmail.com> wrote:
>
> On Mon, Oct 9, 2023 at 3:32 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
>
> In v1, I used the same pattern as on the CREATE SUBSCRIPTION page,
> which doesn't look like those...
>

Yeah, I think it would have been better if we used params in the
CREATE SUBSCRIPTION page as well. I don't know if it is a good idea to
do now with this patch but it makes sense to be consistent. What do
you think?

> ~~~
>
> The "Parameters" section describes some things that really are parameters:
>
> e.g.
> "sql-altersubscription-name"
> "sql-altersubscription-new-owner"
> "sql-altersubscription-new-name">
>
> I agree, emphasising that those ones are parameters is better. Changed
> like this in v2.
>
> "sql-altersubscription-params-name"
> "sql-altersubscription-params-new-owner"
> "sql-altersubscription-params-new-name">
>
> ~
>
> But, the "Parameters" section also describes other SQL syntax clauses
> which are not really parameters at all.
>
> e.g.
> "sql-altersubscription-refresh-publication"
> "sql-altersubscription-enable"
> "sql-altersubscription-disable"
>
> So I felt those ones are more intuitive left as they are  -- e.g.,
> instead of having ids/linkends like:
>
> "sql-altersubscription-params-refresh-publication"
> "sql-altersubscription-params-enable"
> "sql-altersubscription-params-disable"
>

I checked alter_role.sgml which has similar mixed usage and it is
using 'params' consistently in all cases. So, I would suggest
following a similar style here.

-- 
With Regards,
Amit Kapila.

  11. Re: PGDOCS - add more links in the pub/sub reference pages

    Peter Smith <smithpb2250@gmail.com> — 2023-10-13T03:32:52Z

    On Thu, Oct 12, 2023 at 3:44 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Oct 9, 2023 at 12:15 PM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > On Mon, Oct 9, 2023 at 3:32 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> >
> > In v1, I used the same pattern as on the CREATE SUBSCRIPTION page,
> > which doesn't look like those...
> >
>
> Yeah, I think it would have been better if we used params in the
> CREATE SUBSCRIPTION page as well. I don't know if it is a good idea to
> do now with this patch but it makes sense to be consistent. What do
> you think?
>

OK, I have given those changes as separate patches:
- 0002 (changes the CREATE PUBLICATION parameter ids)
- 0003 (changes CREATE SUBSCRIPTION parameter ids)

> > ~~~
> >
> > The "Parameters" section describes some things that really are parameters:
> >
> > e.g.
> > "sql-altersubscription-name"
> > "sql-altersubscription-new-owner"
> > "sql-altersubscription-new-name">
> >
> > I agree, emphasising that those ones are parameters is better. Changed
> > like this in v2.
> >
> > "sql-altersubscription-params-name"
> > "sql-altersubscription-params-new-owner"
> > "sql-altersubscription-params-new-name">
> >
> > ~
> >
> > But, the "Parameters" section also describes other SQL syntax clauses
> > which are not really parameters at all.
> >
> > e.g.
> > "sql-altersubscription-refresh-publication"
> > "sql-altersubscription-enable"
> > "sql-altersubscription-disable"
> >
> > So I felt those ones are more intuitive left as they are  -- e.g.,
> > instead of having ids/linkends like:
> >
> > "sql-altersubscription-params-refresh-publication"
> > "sql-altersubscription-params-enable"
> > "sql-altersubscription-params-disable"
> >
>
> I checked alter_role.sgml which has similar mixed usage and it is
> using 'params' consistently in all cases. So, I would suggest
> following a similar style here.
>

As you wish. Done that way in patch 0001.

~~

PSA the v5 patches.

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

  12. Re: PGDOCS - add more links in the pub/sub reference pages

    Peter Smith <smithpb2250@gmail.com> — 2023-10-16T00:45:30Z

    Thanks for pushing the 0001 patch! I am unsure what the decision was
for the remaining patches, but anyway, here they are again (rebased).

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

  13. Re: PGDOCS - add more links in the pub/sub reference pages

    Amit Kapila <amit.kapila16@gmail.com> — 2023-10-30T09:15:44Z

    On Mon, Oct 16, 2023 at 6:15 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> Thanks for pushing the 0001 patch! I am unsure what the decision was
> for the remaining patches, but anyway, here they are again (rebased).
>

To keep the link names the same in logical replication-related
commands, I have pushed your patch.

-- 
With Regards,
Amit Kapila.

  14. Re: PGDOCS - add more links in the pub/sub reference pages

    Peter Smith <smithpb2250@gmail.com> — 2023-10-30T21:53:05Z

    Thanks for pushing!

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