Re: Added schema level support for publication.

On Wed, Sep 29, 2021 at 3:16 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> 4.
> + /*
> + * 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,
> + errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> + errmsg("cannot move table \"%s\" to schema \"%s\"",
> + RelationGetRelationName(rel), stmt->newschema),
> + errdetail("Altering table will result in having schema \"%s\" and
> same schema's table \"%s\" in the publication \"%s\" which is not
> supported.",
> + stmt->newschema,
> + RelationGetRelationName(rel),
> + get_publication_name(pubid, false)));
> + }
> + }
>
> Let's slightly change the comment as: "Check that setting the relation
> to a different schema won't result in the publication having schema
> and the same schema's table." and errdetail as: "The schema \"%s\" and
> same schema's table \"%s\" cannot be part of the same publication
> \"%s\"."
>

Since this code is in AlterTableNamespace() and the relation being
checked may or may not be part of a publication, I'd use "a
publication" instead of "the publication" in the comment.
Also, I'd say that we're doing the check because the mentioned
combination is not supported.

i.e. "Check that setting the relation to a different schema won't
result in a publication having both a schema and the same schema's
table, as this is not supported."

Regards,
Greg Nancarrow
Fujitsu Australia