On Mon, 5 Sept 2022 at 15:10, Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Sep 5, 2022 at 9:47 AM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > Here are my review comments for v45-0001:
> >
> > ======
> >
> > 1. doc/src/sgml/logical-replication.sgml
> >
> > <para>
> > To find which tables might potentially include non-local origins (due to
> > other subscriptions created on the publisher) try this SQL query:
> > <programlisting>
> > SELECT DISTINCT N.nspname AS schemaname, C.relname AS tablename
> > FROM pg_publication P,
> > LATERAL pg_get_publication_tables(P.pubname) GPT
> > LEFT JOIN pg_subscription_rel PS ON (GPT.relid = PS.srrelid),
> > pg_class C JOIN pg_namespace N ON (N.oid = C.relnamespace)
> > WHERE C.oid = GPT.relid AND PS.srrelid IS NOT NULL AND
> > P.pubname IN (pub-names);
> > </programlisting></para>
> >
> > 1a.
> > To use "<pub-names>" with the <> then simply put meta characters in the SGML.
> > e.g.
> > <pub-names>
> >
> > ~
> >
> > 1b.
> > The patch forgot to add the SQL "#" instruction as suggested by my
> > previous comment (see [1] #3b)
> >
> > ~~~
> >
> > 2.
> >
> > <sect1 id="preventing-inconsistencies-for-copy_data-origin">
> > <title>Preventing Data Inconsistencies for copy_data, origin=NONE</title>
> >
> > The title is OK, but I think this should all be a <sect2> sub-section
> > of "31.2 Subscription"
> >
> > ======
> >
>
> It is recommended to create the subscription
> + using <literal>enabled=false</literal>, so that if the origin WARNING occurs
> + no copy has happened yet. Otherwise some corrective steps might be needed to
> + remove any unwanted data that got copied.
>
> I am not completely sure of this part of the docs as this can add
> additional steps for users while working on subscriptions even when
> the same is not required. I suggest for now we can remove this part.
> Later based on some feedback on this feature, we can extend the docs
> if required.
Modified
> Also, instead of having it as a separate section, let's keep this as
> part of create_subscription.sgml
Modified
> *
> + errhint("Verify that initial data copied from the publisher tables
> did not come from other origins. Some corrective action may be
> necessary."));
>
> The second sentence in this message doesn't seem to be required.
Modified
Thanks for the comments, the v46 patch at [1] has the changes for the same.
[1] - https://www.postgresql.org/message-id/CALDaNm3TTGdCCkeDsN8hqtF_2z-8%2B%3D3tc9Gh5xOKAQ_BRMCkMA%40mail.gmail.com
Regards,
Vignesh
