Re: Added schema level support for publication.

On Wed, Sep 29, 2021 at 9:07 AM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:
>
> On Tues, Sep 28, 2021 10:46 PM vignesh C <vignesh21@gmail.com> wrote:
> > Attached v34 patch has the changes for the same.
>
> Thanks for updating the patch.
> Here are a few comments.
>
> 1)
> + *             ALL TABLES IN SCHEMA schema [[, ...]
>
> [[ -> [

Modified

> 2)
> +       /* ALTER PUBLICATION ... ADD/DROP TABLE/ALL TABLES IN SCHEMA parameters */
>
> The two '/' seems a bit unclear and it doesn't mention the SET case.
> Maybe we can write like:
>
> /* parameters used for ALTER PUBLICATION ... ADD/DROP/SET publication objects */

Modified

> 3)
> +       /*
> +        * Check if setting the relation to a different schema will result in the
> +        * publication having schema and same schema's table in the publication.
> +        */
> +       if (stmt->objectType == OBJECT_TABLE)
> +       {
> +               ListCell   *lc;
> +               List       *schemaPubids = GetSchemaPublications(nspOid);
> +               foreach(lc, schemaPubids)
> +               {
> +                       Oid             pubid = lfirst_oid(lc);
> +                       if (list_member_oid(GetPublicationRelations(pubid, PUBLICATION_PART_ALL),
> +                                                               relid))
> +                               ereport(ERROR,
>
> How about we check this case like the following ?
>
> List       *schemaPubids = GetSchemaPublications(nspOid);
> List       *relPubids = GetRelationPublications(RelationGetRelid(rel));
> if (list_intersection(schemaPubids, relPubids))
>         ereport(ERROR, ...

Modified it with slight changes without using list_intersection.

These comments are handled in the v35 version patch attached at [1]
[1] - https://www.postgresql.org/message-id/CALDaNm2yJOEPCqR%3DgTMEwveJujH9c9_z4LhKmk2T3vZH7T1DLQ%40mail.gmail.com

Regards,
VIgnesh