Re: Added schema level support for publication.

On Fri, Jul 30, 2021 at 12:12 PM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:
>
> On July 28, 2021 6:44 PM vignesh C <vignesh21@gmail.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