Re: Added schema level support for publication.

On Fri, Jul 30, 2021 at 12:12 PM houzj(dot)fnst(at)fujitsu(dot)com
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On July 28, 2021 6:44 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> > Thanks for the patch, I have merged the changes. Attached v16 patch has the
> > fixes for the same.
>
> Thanks for the new version patches.
> Here are a few comments:
>
> 1)
>
> + /* Identify which schemas should be dropped */
> + foreach(oldlc, oldschemaids)
> + {
> + Oid oldschemaid = lfirst_oid(oldlc);
> +
> + if (!list_member_oid(schemaoidlist, oldschemaid))
> + delschemas = lappend_oid(delschemas, oldschemaid);
> + }
> +
>
> We can use list_difference here to simplify the code.

Modified.

> 2)
>
> +
> + /* Filter out duplicates if user specifies "sch1, sch1" */
> + if (list_member_oid(schemaoidlist, schemaoid))
> + continue;
> +
> + schemaoidlist = lappend_oid(schemaoidlist, schemaoid);
>
> It might be more concise to use list_append_unique_oid() here.

Modified.

> 3)
>
> + <para>
> + Create a publication that publishes all changes for all the tables present in
> +the schema "production":
> +<programlisting>
>
> The second line seems not aligned.
> After:
> + <para>
> + Create a publication that publishes all changes for all the tables present in
> + the schema "production":
> +<programlisting>

Modified.

> 4)
>
> + resetPQExpBuffer(query);
> +
> + /* Get the publication membership for the schema */
> + appendPQExpBuffer(query,
> + "SELECT ps.psnspcid, ps.oid, p.pubname, p.oid AS pubid "
> + "FROM pg_publication_schema ps, pg_publication p "
> + "WHERE ps.psnspcid = '%u' "
> + "AND p.oid = ps.pspubid",
> + nsinfo->dobj.catId.oid);
>
> It seems we can use printfPQExpBuffer() here which is a convenience routine
> that does the same thing as resetPQExpBuffer() followed by appendPQExpBuffer().

Modified.

Thanks for the comments, attached v17 patches has the fixes for the same.

Regards,
Vignesh