On Wed, May 18, 2022 at 8:30 AM shiy.f...@fujitsu.com
<shiy.f...@fujitsu.com> wrote:
>
> On Sat, May 14, 2022 9:33 PM vignesh C <vignes...@gmail.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".
Modified it
> 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().)
Ideally this scenario should not happen, but if it happens I felt we
should throw an error in this case.
The v6 patch attached at [1] has the changes for the same.
[1] -
https://www.postgresql.org/message-id/CALDaNm0iZZDB300Dez_97S8G6_RW5QpQ8ef6X3wq8tyK-8wnXQ%40mail.gmail.com
Regards,
Vignesh
