| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | vignesh C <vignesh21(at)gmail(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Review items for EXCEPT TABLE publication |
| Date: | 2026-09-10 07:53:45 |
| Message-ID: | D700C437-EDF2-4442-90AD-E1D45F9E50AE@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> 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?
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andrey Borodin | 2026-09-10 08:01:33 | Re: Possible race condition in pg_basebackup |
| Previous Message | Michael Paquier | 2026-09-10 07:51:54 | Re: Support for 8-byte TOAST values, round two |