Re: table-write trigger can bypass ATPrepChangePersistence

[Ajin Cherian]

On Thu, Sep 17, 2026 at 2:49 PM Hayato Kuroda (Fujitsu)
<kuroda.hay...@fujitsu.com> wrote:
>
> Hi hackers,
>
> While working on other projects, I found an issue $SUBJECT. Below describes 
> the
> exact problem, reproducer, and fix idea.
>
> Problem
> ======
> Unlogged tables cannot be included and excluded in a publication. When SET 
> UNLOGGED
> command is executed, validations are done in ATPrepChangePersistence() and the
> backend raises an ERROR. However, rewrite-table event trigger can be fired 
> after
> the validation, and publication commands can be run at that time. Such 
> commands
> would bypass the validation thus unlogged tables could be in the 
> pg_publication_rel.
>

Hello Kuroda-san,

Thanks for the patch!
While testing the patch, I found a similar issue in the related area
which this patch can fix with a small change.

Here is the problem. While ATRewriteTables() rewrites a table, it
evaluates DEFAULT expressions for each row. A DEFAULT expression can
call a function. That function can run DDL. This DDL can run after
your recheck. So it can still bypass the check.

For example:

CREATE TABLE pub_rewrite (a int);
INSERT INTO pub_rewrite VALUES (1);

CREATE PUBLICATION pub;

CREATE FUNCTION add_to_pub_during_rewrite() RETURNS int
LANGUAGE plpgsql VOLATILE AS $$
BEGIN
  EXECUTE 'ALTER PUBLICATION pub ADD TABLE pub_rewrite';
  RETURN 1;
END;
$$;

-- Should fail, but succeeds
ALTER TABLE pub_rewrite
  SET UNLOGGED,
  ADD COLUMN b int DEFAULT add_to_pub_during_rewrite();

SELECT c.relpersistence, pr.prexcept
  FROM pg_class c JOIN pg_publication_rel pr ON pr.prrelid = c.oid
  WHERE c.oid = 'pub_rewrite'::regclass;
--  relpersistence | prexcept
-- ----------------+----------
--  u              | f
-- (1 row)

Now for a possible fix. Your patch puts the recheck right after the
event trigger fires. That is inside the per-table loop in
ATRewriteTables(). The DEFAULT expression runs later in that same
loop, during the actual rewrite of that table in ATRewriteTable(). So
the recheck happens too early for this case.

A better place is after the whole loop finishes. This means every
table has already been rewritten. All DEFAULT and CHECK functions have
already run. Only then do we check persistence again, for every table
in the list.

regards,
Ajin Cherian
Fujitsu Australia