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