Subject: DOCS: pg_createsubscriber wrong link?

From: Peter Smith <[email-masked]>
Date: Fri, 13 Dec 2024 16:27:38 +1100

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.

~~

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

---

From: vignesh C <[email-masked]>
Date: Fri, 13 Dec 2024 15:01:34 +0530

> 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

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

---

From: Peter Smith <[email-masked]>
Date: Mon, 16 Dec 2024 08:22:39 +1100

> I also felt that the link was directed to the wrong page.

Thanks.

> linkend="sql-createsubscription-params-with-copy-data">initial data

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

---

From: Amit Kapila <[email-masked]>
Date: Mon, 16 Dec 2024 16:26:38 +0530

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

---

From: Pavel Luzanov <[email-masked]>
Date: Mon, 16 Dec 2024 18:42:07 +0300

On 16.12.2024 13:56, Amit Kapila wrote:
> On Fri, Dec 13, 2024 at 10:58 AM Peter Smith<[email-masked]>  wrote:

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

---

From: vignesh C <[email-masked]>
Date: Tue, 17 Dec 2024 14:44:15 +0530

> the reader would have no clue about the link target.

Thanks for the updated version. The v2 version patch looks great and
applies cleanly to both the HEAD and PG17 branches.

Regards,
Vignesh