MIME-Version: 1.0
References: 
 <CALDaNm3=JrucjhiiwsYQw5-PGtBHFONa6F7hhWCXMsGvh=tamA@mail.gmail.com>
 <CALj2ACVOzhs+BD+abFV2x4oKJdsDNd6SgsE7r8UjnZDCKGEckA@mail.gmail.com>
 <CAA4eK1K6Kr88d2S0zFdHRMyuoaZeNh+ktU+oigmCuD09_x_-+g@mail.gmail.com>
 <CAHut+PsvC-NezO3MJkdyEz=G1QRje2LntjwhQiEeVbmhOQuBMA@mail.gmail.com>
 <CALDaNm18VH2j8cTqfELHQ=0ZNognbGBhbHPteJenWQC6C2dueQ@mail.gmail.com>
 <CALDaNm0k_0Ccj47wzJzzPFwgQB7w=R5+Q2_nSqYrmMmjhmcRUw@mail.gmail.com>
 <CAHut+Pv_0DwyWoGQNMF+G2AGqMuJTzWQKRtmxaC+=zLTPL-Zkw@mail.gmail.com>
 <CALDaNm2-GJt2HsYTkLqQ=ecm=R-vOBw1=aM_d2EiYbz39x_cTQ@mail.gmail.com>
 <TYAPR01MB6315FF450C5D10253BA7C7FFFDD19@TYAPR01MB6315.jpnprd01.prod.outlook.com>
In-Reply-To: 
 <TYAPR01MB6315FF450C5D10253BA7C7FFFDD19@TYAPR01MB6315.jpnprd01.prod.outlook.com>
From: vignesh C <vignesh21@gmail.com>
Date: Wed, 18 May 2022 23:19:01 +0530
Message-ID: 
 <CALDaNm3PcrbmLcgBi38oL_268TnERWBG1c5kFDuNViTszP_K2Q@mail.gmail.com>
Subject: Re: Skipping schema changes in publication
To: "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>
Cc: Peter Smith <smithpb2250@gmail.com>,
 Amit Kapila <amit.kapila16@gmail.com>,
	Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CALDaNm3PcrbmLcgBi38oL_268TnERWBG1c5kFDuNViTszP_K2Q%40mail.gmail.com>
Precedence: bulk

On Wed, May 18, 2022 at 8:30 AM shiy.fnst@fujitsu.com
<shiy.fnst@fujitsu.com> wrote:
>
> On Sat, May 14, 2022 9:33 PM vignesh C <vignesh21@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