Re: PGDOCS - add more links in the pub/sub reference pages
Thanks for pushing! == Kind Regards, Peter Smith. Fujitsu Australia
Re: PGDOCS - add more links in the pub/sub reference pages
On Mon, Oct 16, 2023 at 6:15 AM Peter Smith 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.
Re: PGDOCS - add more links in the pub/sub reference pages
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 v6-0002-Change-ids-for-CREATE-SUBSCRIPTION-parameters.patch Description: Binary data v6-0001-Change-ids-for-CREATE-PUBLICATION-parameters.patch Description: Binary data
Re: PGDOCS - add more links in the pub/sub reference pages
On Thu, Oct 12, 2023 at 3:44 PM Amit Kapila wrote: > > On Mon, Oct 9, 2023 at 12:15 PM Peter Smith wrote: > > > > On Mon, Oct 9, 2023 at 3:32 PM Amit Kapila 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 v5-0001-Add-more-pub-sub-links.patch Description: Binary data v5-0002-Change-ids-for-CREATE-PUBLICATION-parameters.patch Description: Binary data v5-0003-Change-ids-for-CREATE-SUBSCRIPTION-parameters.patch Description: Binary data
Re: PGDOCS - add more links in the pub/sub reference pages
On Mon, Oct 9, 2023 at 12:15 PM Peter Smith wrote: > > On Mon, Oct 9, 2023 at 3:32 PM Amit Kapila 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.
Re: PGDOCS - add more links in the pub/sub reference pages
On Tue, Oct 10, 2023 at 5:10 PM vignesh C 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 CREATE SUBSCRIPTION or > the last invocation of REFRESH PUBLICATION. 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 CREATE SUBSCRIPTION and > it > is dropped automatically when the subscription is dropped using > DROP SUBSCRIPTION. 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 ALTER SUBSCRIPTION 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 ALTER SUBSCRIPTION ... DROP PUBLICATION 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 >ALTER SUBSCRIPTION ... SKIP with the finish LSN >(i.e., LSN 0/14C0378). > 2.e)Before using this function, the subscription needs to be > disabled temporarily >either by ALTER SUBSCRIPTION ... DISABLE 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 v4-0001-Add-more-pub-sub-links.patch Description: Binary data
Re: PGDOCS - add more links in the pub/sub reference pages
On Tue, Oct 10, 2023 at 11:33 PM Amit Kapila wrote: > > On Tue, Oct 10, 2023 at 11:40 AM vignesh C wrote: > > > > On Tue, 10 Oct 2023 at 08:47, Peter Smith 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 CREATE SUBSCRIPTION or > > the last invocation of REFRESH PUBLICATION. > > > > 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
Re: PGDOCS - add more links in the pub/sub reference pages
On Tue, Oct 10, 2023 at 11:40 AM vignesh C wrote: > > On Tue, 10 Oct 2023 at 08:47, Peter Smith 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 CREATE SUBSCRIPTION or > the last invocation of REFRESH PUBLICATION. > 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.
Re: PGDOCS - add more links in the pub/sub reference pages
On Tue, 10 Oct 2023 at 08:47, Peter Smith wrote: > > On Tue, Oct 10, 2023 at 11:46 AM vignesh C 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 ALTER SUBSCRIPTION ... DISABLE, and then > > disassociate it from the replication slot by executing ALTER > > SUBSCRIPTION ... SET (slot_name = NONE). > > > > 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 CREATE SUBSCRIPTION or the last invocation of REFRESH PUBLICATION. 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 CREATE SUBSCRIPTION and it is dropped automatically when the subscription is dropped using DROP SUBSCRIPTION. 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 ALTER SUBSCRIPTION 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 ALTER SUBSCRIPTION ... DROP PUBLICATION to remove one of the offending publications and add it again. 2.d) The transaction that produced the conflict can be skipped by using ALTER SUBSCRIPTION ... SKIP with the finish LSN (i.e., LSN 0/14C0378). 2.e)Before using this function, the subscription needs to be disabled temporarily either by ALTER SUBSCRIPTION ... DISABLE or, Regards, Vignesh
Re: PGDOCS - add more links in the pub/sub reference pages
On Tue, Oct 10, 2023 at 11:46 AM vignesh C 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 ALTER SUBSCRIPTION ... DISABLE, and then > disassociate it from the replication slot by executing ALTER > SUBSCRIPTION ... SET (slot_name = NONE). > Hi Vignesh. Thanks for the review comments. Modified as suggested. PSA v3. == Kind Regards, Peter Smith. Fujitsu Australia v3-0001-Add-more-links.patch Description: Binary data
Re: PGDOCS - add more links in the pub/sub reference pages
On Mon, 9 Oct 2023 at 12:18, Peter Smith wrote: > > On Mon, Oct 9, 2023 at 3:32 PM Amit Kapila wrote: > > > > On Fri, Oct 6, 2023 at 12:15 PM Peter Smith 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" > > > > > > > - > > + > > new_owner > > > > > > @@ -281,7 +281,7 @@ ALTER SUBSCRIPTION > class="parameter">name RENAME TO < > > > > > > > > - > > + > > new_name > > > > > > 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 ALTER SUBSCRIPTION ... DISABLE, and then disassociate it from the replication slot by executing ALTER SUBSCRIPTION ... SET (slot_name = NONE). Regards, Vignesh
Re: PGDOCS - add more links in the pub/sub reference pages
On Mon, Oct 9, 2023 at 3:32 PM Amit Kapila wrote: > > On Fri, Oct 6, 2023 at 12:15 PM Peter Smith 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" > > > > - > + > new_owner > > > @@ -281,7 +281,7 @@ ALTER SUBSCRIPTION class="parameter">name RENAME TO < > > > > - > + > new_name > > 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 v2-0001-Add-more-link.patch Description: Binary data
Re: PGDOCS - add more links in the pub/sub reference pages
On Fri, Oct 6, 2023 at 12:15 PM Peter Smith 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" > - + new_owner @@ -281,7 +281,7 @@ ALTER SUBSCRIPTION name RENAME TO < - + new_name 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.
