Re: Skipping schema changes in publication

From: vignesh C <vignesh21(at)gmail(dot)com>
To: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
Cc: Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, YeXiu <1518981153(at)qq(dot)com>, Ian Lawrence Barwick <barwick(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Skipping schema changes in publication
Date: 2026-02-16 03:20:36
Message-ID: CALDaNm0MpQPoU+C-Jt6BDOfFSFoU=DsFaV-Ohs2nkGnGfLiCcQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, 12 Feb 2026 at 13:58, David G. Johnston
<david(dot)g(dot)johnston(at)gmail(dot)com> wrote:
>
> On Wed, Feb 11, 2026 at 11:58 PM Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> wrote:
>>
>>
>> We have addressed the comments in the latest v43 patch.
>
>
> Non-comprehensive review.
>
> Shouldn't the early exits look like:
>
> <begin function>
> if (list_length(publications) < 2)
> return;
>
> perform query here, capture except_publications
>
> if (list_length(except_publications) < 2)
> return;
>
> construct error message and ereport
> <end function>

Modified

>
> + if (publication_has_any_exception(puboid))
> + {
> + except_pub_names = lappend(except_pub_names,
> + makeString(pubform->pubname.data));
> + has_any_exclusion = true;
> + except_pub_id = pubform->oid;
> + }
>
> Either rename has_any_exclusion to has_any_exception or, given how ambiguous that reads in code, maybe standardize on calling these exclusions throughout the code and either just accept we've chosen EXCEPT for the syntax for good reasons or consider whether to EXCLUDING (table1, table2) would be a better choice.

Changed it to has_any_exception

> + errmsg("could not get non excluded table list for table \"%s.%s\" from publisher: %s", -- triple negative; try to avoid "non excluded" as a term - those are "included".
>
> pg_get_publication_effective_tables - the second input argument is an array of publication names so the singular form here is a bit misleading. Should this be subscription-oriented? Otherwise, pg_get_tables_from_publications seems like a more accurate name. "effective" or "only the included ones" seems reasonable to imply.

We have used similarly in the existing pg_get_publication_tables, I
felt the existing might be ok to keep it consistent unless there is a
better consistent name.

> Related, the check in there for "does at least one publication have an exclusion list" makes sense but feels awfully similar to the check for "at most one publication has an exclusion list"...too late for me to figure out what if anything to do make/do about it though.

Ideally this is not expected to happen. But under rare cases it can
happen if the publication is dropped/recreated with except after
creating a subscription. It is better to throw an error in these cases
as we do not allow multiple publications except tables.

Thanks for the comments, the attached v44 version patch has the
changes for the same.

Regards,
Vignesh

Attachment Content-Type Size
v44-0001-Skip-publishing-the-tables-specified-in-EXCEPT-T.patch application/octet-stream 107.0 KB
v44-0002-Extended-tests-for-EXCEPT-TABLE-patch.patch application/octet-stream 4.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Smith 2026-02-16 03:39:29 DOCS - pg_walsummary typo?
Previous Message Hayato Kuroda (Fujitsu) 2026-02-16 03:10:46 RE: BUG: Former primary node might stuck when started as a standby