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

From: Peter Smith <[email-masked]>
Date: Mon, 9 Oct 2023 17:45:31 +1100

On Mon, Oct 9, 2023 at 3:32 PM Amit Kapila <[email-masked]> wrote:
>
> On Fri, Oct 6, 2023 at 12:15 PM Peter Smith <[email-masked]> 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

---

From: vignesh C <[email-masked]>
Date: Tue, 10 Oct 2023 06:16:15 +0530

> "sql-altersubscription-params-disable"

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

---

From: Peter Smith <[email-masked]>
Date: Tue, 10 Oct 2023 14:17:25 +1100

> SUBSCRIPTION ... SET (slot_name = NONE)</literal>.

Hi Vignesh. Thanks for the review comments.

Modified as suggested.

PSA v3.

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

---

From: vignesh C <[email-masked]>
Date: Tue, 10 Oct 2023 11:40:46 +0530

> Hi Vignesh. Thanks for the review comments.

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

---

From: Amit Kapila <[email-masked]>
Date: Tue, 10 Oct 2023 18:03:05 +0530

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

---

From: Amit Kapila <[email-masked]>
Date: Thu, 12 Oct 2023 10:14:35 +0530

> 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?

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

---

From: Peter Smith <[email-masked]>
Date: Fri, 13 Oct 2023 14:32:52 +1100

> do now with this patch but it makes sense to be consistent. What do

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

> following a similar style here.

As you wish. Done that way in patch 0001.

~~

PSA the v5 patches.

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

---

From: Peter Smith <[email-masked]>
Date: Mon, 16 Oct 2023 11:45:30 +1100

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

---

From: Amit Kapila <[email-masked]>
Date: Mon, 30 Oct 2023 14:45:44 +0530

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