Horton

Re: Skipping schema changes in publication

vignesh C <vignesh21@gmail.com>

View thread
From: vignesh C <vignesh21@gmail.com>
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>
Date: 2022-05-18T17:49:01Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix miscellaneous issues in EXCEPT publication clause.

  2. Change syntax of EXCEPT TABLE clause in publication commands.

  3. Add support for EXCEPT TABLE in ALTER PUBLICATION.

  4. Allow table exclusions in publications via EXCEPT TABLE.

  5. Add wait_for_subscription_sync for TAP tests.

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