Обсуждение: PGDOCS - add more links in the pub/sub reference pages
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
Вложения
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.
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
Вложения
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
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
Вложения
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
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.
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
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
Вложения
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.
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
Вложения
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
Вложения
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.
Thanks for pushing! ====== Kind Regards, Peter Smith. Fujitsu Australia