On Tue, 6 Sept 2022 at 09:31, shiy.f...@fujitsu.com <shiy.f...@fujitsu.com> wrote: > > On Tue, Sep 6, 2022 11:14 AM vignesh C <vignes...@gmail.com> wrote: > > > > Thanks for the comments, the attached patch has the changes for the same. > > > > Thanks for updating the patch. Here are some comments. > > 1. > + if (subrel_count) > + { > + /* > + * In case of ALTER SUBSCRIPTION ... REFRESH, > subrel_local_oids > + * contains the list of relation oids that are already > present on the > + * subscriber. This check should be skipped for these tables. > + */ > + appendStringInfo(&cmd, "AND C.oid NOT IN (\n" > + "SELECT C.oid \n" > + "FROM pg_class C\n" > + " JOIN pg_namespace N ON > N.oid = C.relnamespace\n" > + "WHERE "); > + get_skip_tables_str(subrel_local_oids, subrel_count, &cmd); > + appendStringInfo(&cmd, ")"); > + } > > I think we can skip the tables without using a subquery. See the SQL below. > Would it be better? > > SELECT DISTINCT P.pubname AS pubname > 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 ('p1', > 'p3') > AND NOT (N.nspname = 'public' AND C.relname = 'tbl') > AND NOT (N.nspname = 'public' AND C.relname = 't1');
Modified > 2. > + When using a subscription parameter combination of > + <literal>copy_data=true</literal> and <literal>origin=NONE</literal>, the > > Could we use "copy_data = true" and "origin = NONE" (add two spaces around the > equals sign)? I think it looks clearer that way. And it is consistent with > other > places in this file. Modified Thanks for the comments, the attached v47 patch has the changes for the same. Regards, Vignesh
v47-0001-Raise-a-warning-if-there-is-a-possibility-of-dat.patch
Description: Binary data
v47-0002-Document-the-steps-for-replication-between-prima.patch
Description: Binary data