Horton

Thread

Commits

  1. Doc: Fix the wrong link on pg_createsubscriber page.

  2. Doc: Update link to the mentioned subsection

  1. DOCS: pg_createsubscriber wrong link?

    Peter Smith <smithpb2250@gmail.com> — 2024-12-13T05:27:38Z

    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

  2. Re: DOCS: pg_createsubscriber wrong link?

    vignesh C <vignesh21@gmail.com> — 2024-12-13T09:31:34Z

    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

  3. Re: DOCS: pg_createsubscriber wrong link?

    Peter Smith <smithpb2250@gmail.com> — 2024-12-15T21:22:39Z

    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

  4. Re: DOCS: pg_createsubscriber wrong link?

    Amit Kapila <amit.kapila16@gmail.com> — 2024-12-16T10:56:38Z

    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.

  5. Re: DOCS: pg_createsubscriber wrong link?

    Pavel Luzanov <p.luzanov@postgrespro.ru> — 2024-12-16T15:42:07Z

    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

  6. Re: DOCS: pg_createsubscriber wrong link?

    Amit Kapila <amit.kapila16@gmail.com> — 2024-12-17T08:47:57Z

    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.

  7. Re: DOCS: pg_createsubscriber wrong link?

    vignesh C <vignesh21@gmail.com> — 2024-12-17T09:14:15Z

    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