RE: Skipping schema changes in publication

On Sat, May 14, 2022 9:33 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> Thanks for the comments, the attached v5 patch has the changes for the
> same. Also I have made the changes for SKIP Table based on the new
> syntax, the changes for the same are available in
> v5-0002-Skip-publishing-the-tables-specified-in-EXCEPT-TA.patch.
>

Thanks for your patch. Here are some comments on v5-0001 patch.

+ Oid relid = lfirst_oid(lc);
+
+ prid = GetSysCacheOid2(PUBLICATIONRELMAP, Anum_pg_publication_rel_oid,
+ ObjectIdGetDatum(relid),
+ ObjectIdGetDatum(pubid));
+ if (!OidIsValid(prid))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("relation \"%s\" is not part of the publication",
+ RelationGetRelationName(rel))));

I think the relation in the error message should be the one whose oid is
"relid", instead of relation "rel".

Besides, I think it might be better not to report an error in this case. If
"prid" is invalid, just ignore this relation. Because in RESET cases, we want to
drop all tables in the publications, and there is no specific table.
(If you agree with that, similarly missing_ok should be set to true when calling
PublicationDropSchemas().)

Regards,
Shi yu