DOCS: pg_createsubscriber wrong link?
Hi,
While reviewing the pg_createsubscriber [1]https://www.postgresql.org/docs/current/app-pgcreatesubscriber.html docs I found a potentially
wrong linkend.
This sentence:
"For smaller databases, initial data synchronization is recommended."
links to [2]https://www.postgresql.org/docs/current/logical-replication-row-filter.html#LOGICAL-REPLICATION-ROW-FILTER-INITIAL-DATA-SYNC ("29.4.5. Initial Data Synchronization").
This seems to have been deliberately changed (commit [3]https://github.com/postgres/postgres/commit/84db9a0eb10dd1dbee6db509c0e427fa237177dc)
FROM: <link linkend="logical-replication">initial data synchronization</link>
TO: <link linkend="logical-replication-row-filter-initial-data-sync">initial
data synchronization</link>
Although the title "Initial Data Synchronization" seems at face value
to be relevant, this particular link target is a sub-section of "Row
Filters", so I don't see why this would be the intended link from the
pg_createsubscriber. AFAICT, the original discussion and commit
message does not explain.
~~
Here is a new patch giving an alternate link which IMO might be more
appropriate.
Thoughts?
======
[1]: https://www.postgresql.org/docs/current/app-pgcreatesubscriber.html
[2]: https://www.postgresql.org/docs/current/logical-replication-row-filter.html#LOGICAL-REPLICATION-ROW-FILTER-INITIAL-DATA-SYNC
[3]: https://github.com/postgres/postgres/commit/84db9a0eb10dd1dbee6db509c0e427fa237177dc
Kind Regards,
Peter Smith.
Fujitsu Australia
Attachments:
v1-0001-DOCS-Fix-wrong-linkend-on-pg_createsubscriber-pag.patchapplication/octet-stream; name=v1-0001-DOCS-Fix-wrong-linkend-on-pg_createsubscriber-pag.patchDownload
From beb779b8108f410b7bf690f9ccfdd68f87e0bc05 Mon Sep 17 00:00:00 2001
From: Peter Smith <peter.b.smith@fujitsu.com>
Date: Fri, 13 Dec 2024 16:14:54 +1100
Subject: [PATCH v1] DOCS: Fix wrong linkend on pg_createsubscriber page.
---
doc/src/sgml/ref/pg_createsubscriber.sgml | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/ref/pg_createsubscriber.sgml b/doc/src/sgml/ref/pg_createsubscriber.sgml
index df1a92b..9ffc7e2 100644
--- a/doc/src/sgml/ref/pg_createsubscriber.sgml
+++ b/doc/src/sgml/ref/pg_createsubscriber.sgml
@@ -70,8 +70,10 @@ PostgreSQL documentation
spent synchronizing data is usually a large amount of changes to be applied
(that were produced during the initial data copy), which increases even
more the time when the logical replica will be available. For smaller
- databases, <link linkend="logical-replication-row-filter-initial-data-sync">
- initial data synchronization</link> is recommended.
+ databases, initial data synchronization is recommended. For details, see the
+ <command>CREATE SUBSCRIPTION</command> <link linkend="sql-createsubscription-params-with-copy-data">
+ <literal>copy_data</literal></link> option.
+
</para>
</refsect1>
--
1.8.3.1
On Fri, 13 Dec 2024 at 10:58, Peter Smith <smithpb2250@gmail.com> wrote:
Hi,
While reviewing the pg_createsubscriber [1] docs I found a potentially
wrong linkend.This sentence:
"For smaller databases, initial data synchronization is recommended."links to [2] ("29.4.5. Initial Data Synchronization").
This seems to have been deliberately changed (commit [3])
FROM: <link linkend="logical-replication">initial data synchronization</link>
TO: <link linkend="logical-replication-row-filter-initial-data-sync">initial
data synchronization</link>Although the title "Initial Data Synchronization" seems at face value
to be relevant, this particular link target is a sub-section of "Row
Filters", so I don't see why this would be the intended link from the
pg_createsubscriber. AFAICT, the original discussion and commit
message does not explain.
I also felt that the link was directed to the wrong page.
Here is a new patch giving an alternate link which IMO might be more
appropriate.
How about we change the below:
more the time when the logical replica will be available. For smaller
- databases, <link linkend="logical-replication-row-filter-initial-data-sync">
- initial data synchronization</link> is recommended.
+ databases, initial data synchronization is recommended. For details, see the
+ <command>CREATE SUBSCRIPTION</command> <link
linkend="sql-createsubscription-params-with-copy-data">
+ <literal>copy_data</literal></link> option.
+
to:
For smaller databases, it is recommended to set up <link
linkend="logical-replication">logical replication</link> with <link
linkend="sql-createsubscription-params-with-copy-data">initial data
synchronization</link>.Regards,
Vignesh
On Fri, Dec 13, 2024 at 8:31 PM vignesh C <vignesh21@gmail.com> wrote:
On Fri, 13 Dec 2024 at 10:58, Peter Smith <smithpb2250@gmail.com> wrote:
Hi,
While reviewing the pg_createsubscriber [1] docs I found a potentially
wrong linkend.This sentence:
"For smaller databases, initial data synchronization is recommended."links to [2] ("29.4.5. Initial Data Synchronization").
This seems to have been deliberately changed (commit [3])
FROM: <link linkend="logical-replication">initial data synchronization</link>
TO: <link linkend="logical-replication-row-filter-initial-data-sync">initial
data synchronization</link>Although the title "Initial Data Synchronization" seems at face value
to be relevant, this particular link target is a sub-section of "Row
Filters", so I don't see why this would be the intended link from the
pg_createsubscriber. AFAICT, the original discussion and commit
message does not explain.I also felt that the link was directed to the wrong page.
Thanks.
Here is a new patch giving an alternate link which IMO might be more
appropriate.How about we change the below: more the time when the logical replica will be available. For smaller - databases, <link linkend="logical-replication-row-filter-initial-data-sync"> - initial data synchronization</link> is recommended. + databases, initial data synchronization is recommended. For details, see the + <command>CREATE SUBSCRIPTION</command> <link linkend="sql-createsubscription-params-with-copy-data"> + <literal>copy_data</literal></link> option. + to: For smaller databases, it is recommended to set up <link linkend="logical-replication">logical replication</link> with <link linkend="sql-createsubscription-params-with-copy-data">initial data synchronization</link>.
Hi Vignesh.
I took parts of your suggestion:
- I changed the 1st sentence's wording as suggested.
- I included a "logical replication" link as suggested, but I put it
earlier, where it was first mentioned on this page.
- I kept my 2nd sentence with the "copy_data" link as-is. That's
because if this docs page was in hardcopy format, doing it your way
the reader would have no clue about the link target.
Please see patch v2.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
Attachments:
v2-0001-DOCS-Fix-wrong-linkend-on-pg_createsubscriber-pag.patchapplication/octet-stream; name=v2-0001-DOCS-Fix-wrong-linkend-on-pg_createsubscriber-pag.patchDownload
From 198a1d092bf1663d65ca8974232cac61f00ee9c0 Mon Sep 17 00:00:00 2001
From: Peter Smith <peter.b.smith@fujitsu.com>
Date: Mon, 16 Dec 2024 08:16:12 +1100
Subject: [PATCH v2] DOCS: Fix wrong linkend on pg_createsubscriber page.
---
doc/src/sgml/ref/pg_createsubscriber.sgml | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/doc/src/sgml/ref/pg_createsubscriber.sgml b/doc/src/sgml/ref/pg_createsubscriber.sgml
index f8a3911..a2388ce 100644
--- a/doc/src/sgml/ref/pg_createsubscriber.sgml
+++ b/doc/src/sgml/ref/pg_createsubscriber.sgml
@@ -49,7 +49,8 @@ PostgreSQL documentation
<para>
<application>pg_createsubscriber</application> creates a new logical
replica from a physical standby server. All tables in the specified
- database are included in the logical replication setup. A pair of
+ database are included in the <link linkend="logical-replication">logical
+ replication</link> setup. A pair of
publication and subscription objects are created for each database. It
must be run at the target server.
</para>
@@ -70,8 +71,11 @@ PostgreSQL documentation
spent synchronizing data is usually a large amount of changes to be applied
(that were produced during the initial data copy), which increases even
more the time when the logical replica will be available. For smaller
- databases, <link linkend="logical-replication-row-filter-initial-data-sync">
- initial data synchronization</link> is recommended.
+ databases, it is recommended to set up logical replication with initial data
+ synchronization. For details, see the <command>CREATE SUBSCRIPTION</command>
+ <link linkend="sql-createsubscription-params-with-copy-data">
+ <literal>copy_data</literal></link> option.
+
</para>
</refsect1>
--
1.8.3.1
On Fri, Dec 13, 2024 at 10:58 AM Peter Smith <smithpb2250@gmail.com> wrote:
While reviewing the pg_createsubscriber [1] docs I found a potentially
wrong linkend.This sentence:
"For smaller databases, initial data synchronization is recommended."links to [2] ("29.4.5. Initial Data Synchronization").
This seems to have been deliberately changed (commit [3])
Yeah, the change made by commit 84db9a0eb1 is wrong and your latest
patch in this thread looks good to me. I am adding Daniel and the
original author to see if they think differently.
--
With Regards,
Amit Kapila.
On 16.12.2024 13:56, Amit Kapila wrote:
On Fri, Dec 13, 2024 at 10:58 AM Peter Smith<smithpb2250@gmail.com> wrote:
While reviewing the pg_createsubscriber [1] docs I found a potentially
wrong linkend.This sentence:
"For smaller databases, initial data synchronization is recommended."links to [2] ("29.4.5. Initial Data Synchronization").
This seems to have been deliberately changed (commit [3])
Yeah, the change made by commit 84db9a0eb1 is wrong and your latest
patch in this thread looks good to me. I am adding Daniel and the
original author to see if they think differently.
It looks like my mistake.
I missed that "Initial Data Synchronization" section is a subsection
of "Row filters". There is no hidden reason other than matching
the link name with the subsection name and my inattention.
v2 of proposed patch looks good to me.
--
Pavel Luzanov
Postgres Professional:https://postgrespro.com
On Mon, Dec 16, 2024 at 9:12 PM Pavel Luzanov <p.luzanov@postgrespro.ru> wrote:
On 16.12.2024 13:56, Amit Kapila wrote:
Yeah, the change made by commit 84db9a0eb1 is wrong and your latest
patch in this thread looks good to me. I am adding Daniel and the
original author to see if they think differently.It looks like my mistake.
I missed that "Initial Data Synchronization" section is a subsection
of "Row filters". There is no hidden reason other than matching
the link name with the subsection name and my inattention.v2 of proposed patch looks good to me.
Thanks for the confirmation. I'll push the patch.
--
With Regards,
Amit Kapila.
On Mon, 16 Dec 2024 at 02:53, Peter Smith <smithpb2250@gmail.com> wrote:
On Fri, Dec 13, 2024 at 8:31 PM vignesh C <vignesh21@gmail.com> wrote:
On Fri, 13 Dec 2024 at 10:58, Peter Smith <smithpb2250@gmail.com> wrote:
Hi,
While reviewing the pg_createsubscriber [1] docs I found a potentially
wrong linkend.This sentence:
"For smaller databases, initial data synchronization is recommended."links to [2] ("29.4.5. Initial Data Synchronization").
This seems to have been deliberately changed (commit [3])
FROM: <link linkend="logical-replication">initial data synchronization</link>
TO: <link linkend="logical-replication-row-filter-initial-data-sync">initial
data synchronization</link>Although the title "Initial Data Synchronization" seems at face value
to be relevant, this particular link target is a sub-section of "Row
Filters", so I don't see why this would be the intended link from the
pg_createsubscriber. AFAICT, the original discussion and commit
message does not explain.I also felt that the link was directed to the wrong page.
Thanks.
Here is a new patch giving an alternate link which IMO might be more
appropriate.How about we change the below: more the time when the logical replica will be available. For smaller - databases, <link linkend="logical-replication-row-filter-initial-data-sync"> - initial data synchronization</link> is recommended. + databases, initial data synchronization is recommended. For details, see the + <command>CREATE SUBSCRIPTION</command> <link linkend="sql-createsubscription-params-with-copy-data"> + <literal>copy_data</literal></link> option. + to: For smaller databases, it is recommended to set up <link linkend="logical-replication">logical replication</link> with <link linkend="sql-createsubscription-params-with-copy-data">initial data synchronization</link>.Hi Vignesh.
I took parts of your suggestion:
- I changed the 1st sentence's wording as suggested.
- I included a "logical replication" link as suggested, but I put it
earlier, where it was first mentioned on this page.
- I kept my 2nd sentence with the "copy_data" link as-is. That's
because if this docs page was in hardcopy format, doing it your way
the reader would have no clue about the link target.Please see patch v2.
Thanks for the updated version. The v2 version patch looks great and
applies cleanly to both the HEAD and PG17 branches.
Regards,
Vignesh