public inbox for [email protected]
help / color / mirror / Atom feedFrom: Peter Smith <[email protected]>
To: vignesh C <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Shlok Kyal <[email protected]>
Cc: Dilip Kumar <[email protected]>
Cc: shveta malik <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: Nisha Moond <[email protected]>
Cc: Ashutosh Sharma <[email protected]>
Cc: David G. Johnston <[email protected]>
Cc: Zhijie Hou (Fujitsu) <[email protected]>
Cc: YeXiu <[email protected]>
Cc: Ian Lawrence Barwick <[email protected]>
Cc: Bharath Rupireddy <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Skipping schema changes in publication
Date: Wed, 22 Apr 2026 12:58:08 +1000
Message-ID: <CAHut+PvPRrm7GLp1i9qTiYqs8Okbh6vkhkkahiM2=Lo6e9qXTA@mail.gmail.com> (raw)
In-Reply-To: <CALDaNm2VghC-CSyS6jwyMMcpwSWjQ09t9AkkL0Sz7dsb1VGF3A@mail.gmail.com>
References: <CAJpy0uB20MhJJEaPJdm31t4fykJ+fChA_76jU2P9HX5knbJvAA@mail.gmail.com>
<CAD21AoCC8XuwfX62qKBSfHUAoww_XB3_84HjswgL9jxQy696yw@mail.gmail.com>
<OS9PR01MB12149EA0C749BC29C7C949E32F544A@OS9PR01MB12149.jpnprd01.prod.outlook.com>
<CAD21AoBbZEshyaK0PeiF_J4_S75EfF=Gcs=C+X-osoVoUnawuQ@mail.gmail.com>
<CAHut+PssG+sHeV+Xo0g=S7xBb9FgDPjHYDR4iSuOdYXDq-Psng@mail.gmail.com>
<CAA4eK1LaSfAG7UAuy1xpnkWKM_YtrPuhbgAxYBFY3Sp_v_KqoQ@mail.gmail.com>
<CAD21AoAb8E8krN63cY_U7RQs9v-zkqUZyKT_UVKDwKfExtvTBg@mail.gmail.com>
<CAA4eK1K1GLR7DXSABayQE+pWM=v1ODD6haPYxuDhAYwJN5gjzg@mail.gmail.com>
<CALDaNm2kvFahDDvdgCNo=Nv-COz_N5Xw8YmzQBN2bd3g=N81fQ@mail.gmail.com>
<CAHut+PsCqTR_kQu5M1TqBjnE6KM5cO22aH8boHfpMa_gSJBmWg@mail.gmail.com>
<CALDaNm2OOgmNOPpABUU+AXzHhfrLG9HMfSd3jfNe=t3dc-kp1Q@mail.gmail.com>
<CAJpy0uCN4gfP7fSt__KdW5wYQ82650Z6L4YLnjRHZTQ1yir1mg@mail.gmail.com>
<CALDaNm32+c6RTE5xR6sJ=MZGgwEtzjkxpov_Hu70MXfbvmN+=Q@mail.gmail.com>
<CAHut+PtQbK9USLepyzArXFoNuLok1MsBu_Jg4UT=koZocombFw@mail.gmail.com>
<CALDaNm1tKuU479T=winBqoMb3MzO3Mta2juk8W3t2R5ps0_zyg@mail.gmail.com>
<CALDaNm3jpYs7ALcU6m5=Li=udidjZoW5dMpyCFs8QHGaf0S8+A@mail.gmail.com>
<CAJpy0uCWS=ybBKG-kRAfdWEe1VBNj+VqpAUUoT8MPaNS7EggiA@mail.gmail.com>
<CAA4eK1LMM-P4NatbkjG-96B7hHC7KYrJ8XTsCZQy0jLO9Qj4Bw@mail.gmail.com>
<CAJpy0uAyf71QSYitBf4WbCYq22HDR6LPdxB12TpTgTRpczwphw@mail.gmail.com>
<CAFiTN-s5PW121mBGKin20YEQpZkWefMehmP=v+0onzEaMQpwdw@mail.gmail.com>
<CAA4eK1LBf5asit18HcqcFinOkdCjD6Lk2Eid9PDhtH6acwYb8w@mail.gmail.com>
<CALDaNm3cdoT58E3QtYCwBbzyxYJjoS2k7Q0EgzR9ta6fyDGHSg@mail.gmail.com>
<CAHut+PthJx_gZJNgF=mWSpkWjQJ58KyhrZ7D7CkX_TVq12wv7A@mail.gmail.com>
<CANhcyEVLp5kbaVR4=nh1jR4YWqv7YpVx_SnYoshbnOrnY79_fg@mail.gmail.com>
<CAA4eK1+4ZNF-MGheeTtYF9TdfNBnKKJ8DivWZsXBnuJVkqfa0g@mail.gmail.com>
<CANhcyEUQvEK+HOH6Y8Fy30fNvC631ZopWKhwgskXjKnuXiGV5Q@mail.gmail.com>
<CAHut+PuUnDZ4ki8nCK6SkHOn8iP6N1Vm24jzWtEqRG9a_GMxrw@mail.gmail.com>
<CALDaNm0wV2Jd558jWG2EWVAjCiuaAEUrP4i2FWBKqob=1Y9-2A@mail.gmail.com>
<CAHut+Pv+sE82few1Chv4wBGnTR548n_FrzGyabL0w_1TOG6GCA@mail.gmail.com>
<CALDaNm1CiBYcteE_jjPA4BPHfX30dg9eTTTkJgkjY5tgE7t=bQ@mail.gmail.com>
<CAHut+PuzgDiBcD6rp_31RzqbGLpMwqGrNKznFUA_fpBpZYPe9Q@mail.gmail.com>
<CAA4eK1LG9ezz2QHMfaAKeWqCaRLRaDtu6-kBgrCRq14UaB3ECA@mail.gmail.com>
<CAHut+PsXKQbyAhQLRHX0aANkJZPWsYpz4TQQba1WFM7dvSwXNg@mail.gmail.com>
<CALDaNm2VghC-CSyS6jwyMMcpwSWjQ09t9AkkL0Sz7dsb1VGF3A@mail.gmail.com>
On Mon, Apr 20, 2026 at 10:13 PM vignesh C <[email protected]> wrote:
>
> Hi,
>
> When changing a table to UNLOGGED, tables that appear in publications
> via EXCEPT clauses (prexcept = true) are currently allowed, but their
> entries remain in pg_publication_rel.
>
> For example:
> postgres=# create table t1(c1 int);
> CREATE TABLE
> postgres=# create publication pub1 for all tables except (table t1);
> CREATE PUBLICATION
> postgres=# alter table t1 set unlogged;
> ALTER TABLE
> postgres=# \d t1
> Unlogged table "public.t1"
> Column | Type | Collation | Nullable | Default
> --------+---------+-----------+----------+---------
> c1 | integer | | |
> Except publications:
> "pub1"
>
> Since UNLOGGED tables are not supported in publications, this leaves
> stale catalog entries. This patch removes such entries from
> pg_publication_rel when the table is changed to UNLOGGED, and emits a
> NOTICE to inform the user.
>
> Another option considered was to throw an error when setting such
> tables to UNLOGGED. However, allowing the operation was preferred,
> since UNLOGGED tables do not generate WAL and are not replicated
> anyway, so blocking the operation would be unnecessarily restrictive.
>
> Attached patch has the changes for the same.
>
> Thoughts?
>
Hi Vignesh -
A couple of review comments for patch v1-0001.
======
Background: I found when altering a published table from LOGGED to UNLOGGED...
- If it was published by "FOR ALL TABLES" then it just becomes
unpublished *silently*.
- Ditto if it was published by "FOR TABLES IN SCHEMA"
- It seems you only got an error on ALTER LOGGED->UNLOGGED when the
published table was specifically published by "FOR TABLE".
======
src/backend/commands/tablecmds.c
1.
+ /* Find all publications associated with the relation. */
+ pubrellist = SearchSysCacheList1(PUBLICATIONRELMAP,
+ ObjectIdGetDatum(RelationGetRelid(rel)));
This comment seems misleading. IIUC, it's only going to find relations
specifically mentioned by name in the command. Any relations that are
associated with the publication due to "FOR ALL TABLES" or "FOR TABLES
IN SCHEMA" are not known.
~~~
2.
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("cannot change table \"%s\" to unlogged because it is part of
a publication",
+ RelationGetRelationName(rel)),
+ errdetail("Unlogged relations cannot be replicated.")))
This does not seem like a very good error message in the first place
(yes, I know this patch did not change it).
e.g.
a) It says it is "part of a publication", but it does not tell the
user which ones. Maybe list them in the message or provide a HINT to
say use \d to show them.
b) It mixes terminology, first calling it a 'table', and then calling
it a 'relation'
~~~
3.
+ ereport(NOTICE,
+ errmsg("relation \"%s\" removed from publication \"%s\" due to being
changed to UNLOGGED",
+ RelationGetRelationName(rel), pubname));
After some consideration, I think that this really should be an ERROR
too, not just a NOTICE.
Otherwise, if the user toggled the excluded table from
LOGGED->UNLOGGED->LOGGED, then the same table would go from being
excluded to being included! The consequences of publishing something
that was originally *specifically* excluded by the user might be very
bad, so IMO it's better to prevent that from happening -- i.e. sharing
the same ERROR code that was there before.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
view thread (259+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Skipping schema changes in publication
In-Reply-To: <CAHut+PvPRrm7GLp1i9qTiYqs8Okbh6vkhkkahiM2=Lo6e9qXTA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox