Re: Review items for EXCEPT TABLE publication

On Thu, 10 Sept 2026 at 14:55, shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> 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?

I don't think this is an issue. Only the top-most ancestor can appear
in the EXCEPT clause, and we skip publishing based on the table's
last_oid. There is also no issue with dump/restore.

Regards,
Vignesh