Re: Review items for EXCEPT TABLE publication

From: shveta malik <shveta(dot)malik(at)gmail(dot)com>
To: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
Cc: vignesh C <vignesh21(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, shveta malik <shveta(dot)malik(at)gmail(dot)com>
Subject: Re: Review items for EXCEPT TABLE publication
Date: 2026-09-10 09:25:23
Message-ID: CAJpy0uBPNze4zokhk02hrN9VRNFLA9+p0RrvQ-JT2zqZ-hSvZQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Sep 10, 2026 at 1:24 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
>
>
> > On Sep 10, 2026, at 13:39, vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > Finding #5: SET UNLOGGED on an excluded table produces an unrestorable
> > catalog state
> >
> > CREATE PUBLICATION ... FOR ALL TABLES EXCEPT (...) correctly rejects
> > unlogged tables. However, ALTER TABLE ... SET UNLOGGED does not
> > perform the same check. Its publication check uses
> > GetRelationIncludedPublications(), which ignores pg_publication_rel
> > rows marked as prexcept.
> > As a result, the following currently succeeds:
> > CREATE TABLE t (a int);
> > CREATE PUBLICATION p FOR ALL TABLES EXCEPT (TABLE t);
> > ALTER TABLE t SET UNLOGGED;
> >
> > This leaves an unlogged table in the publication's EXCEPT list, even
> > though an unlogged table cannot be added to an EXCEPT clause directly.
> > This causes failures when restoring a dump or running pg_upgrade.
> > For example, pg_dump produces:
> > CREATE PUBLICATION pub1 FOR ALL TABLES EXCEPT (TABLE ONLY public.t1)
> > WITH (publish = 'insert, update, delete, truncate');
> > where t1 is an unlogged table. Restoring the dump fails with:
> > ERROR: cannot specify relation "public.t1" in the publication EXCEPT clause
> > DETAIL: This operation is not supported for unlogged tables.
> >
> > A similar error is seen during pg_upgrade:
> > ERROR: cannot specify relation "public.t1" in the publication EXCEPT clause
> > DETAIL: This operation is not supported for unlogged tables.
> >
> > The fix is to reject changing a table to UNLOGGED when it is
> > referenced in a publication's EXCEPT clause, similar to the existing
> > check for tables included in a publication.
> >
> >
> > Regards,
> > Vignesh
> > <v1-0001-Fix-ALTER-PUBLICATION-race-with-concurrent-SET-AL.patch><v1-0005-Prevent-unlogged-tables-in-publication-EXCEPT-cla.patch><v1-0003-Fix-missing-check-in-test_except_root_partition.patch><v1-0002-Fix-ALTER-PUBLICATION-validation-race.patch><v1-0004-Fix-test-to-use-a-fresh-subscription.patch>
>
> For v1-0005, the code change itself looks good to me.
>
> However, I have some concern about the design. Since a table in the EXCEPT list is not published anyway, do we really need to reject SET UNLOGGED? Would it make more sense to remove the table from the EXCEPT list and emit a NOTICE to inform the user?

I had given a similar comment offlist yesterday, but upon rethinking,
I feel automatically removing the table from the EXCEPT list is
slightly riskier even with NOTICE given. The user may later change the
table back to LOGGED, in which case the publication semantics would
have changed silently; the table would now be published(for ALL TABLEs
case) even though the user never changed the publication
configuration.

~~

I think the proposed fix is also incomplete though for partitioned tables:

CREATE TABLE root (a int) PARTITION BY RANGE (a);
CREATE TABLE part1 PARTITION OF root FOR VALUES FROM (1) TO (100);

CREATE PUBLICATION root_pub FOR ALL TABLES EXCEPT (TABLE root);

-- This fails:
ALTER TABLE root SET UNLOGGED;

-- But this succeeds:
ALTER TABLE part1 SET UNLOGGED;

Shouldn't the second command (for partition) fail too?

thanks
Shveta

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Suraj Kharage 2026-09-10 09:28:31 Re: [PATCH] Add support for INSERT ... SET syntax
Previous Message Etsuro Fujita 2026-09-10 09:24:55 Re: postgres_fdw: transaction mode inheritance corner cases