Re: Handle infinite recursion in logical replication setup

On Tue, 6 Sept 2022 at 09:31, shiy(dot)fnst(at)fujitsu(dot)com
<shiy(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Tue, Sep 6, 2022 11:14 AM vignesh C <vignesh21(at)gmail(dot)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