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'); 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. Regards, Shi yu