From 243f08af11ed549b078b82184bc1f2b20aa1b67a Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Wed, 11 Nov 2026 22:57:03 +0000 Subject: [PATCH v4 4/4] Don't undo table_rewrite event trigger changes in ALTER TABLE Previously, ALTER TABLE phase 3 looked up the tablespace, access method and persistence of a table before firing the table_rewrite event trigger, and rewrote the table with those values. If the trigger changed any of them on the same table, the rewrite silently reverted the change. For example, a trigger that ran ALTER TABLE ... SET LOGGED on a table being rewritten by ALTER COLUMN TYPE left the table unlogged again. If the trigger also added a foreign key from a permanent table to it, this bypassed the check that permanent tables do not reference unlogged ones. Fix this by firing the event trigger before looking at the relation. Backpatch to all supported versions. --- src/backend/commands/tablecmds.c | 32 +++++++++++---------- src/test/regress/expected/event_trigger.out | 23 +++++++++++++++ src/test/regress/sql/event_trigger.sql | 19 ++++++++++++ 3 files changed, 59 insertions(+), 15 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 53a2b3dc659..72b73fad664 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -5976,6 +5976,23 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode, Oid NewTableSpace; char persistence; + /* + * Fire off an Event Trigger now, before actually rewriting the + * table. This must happen before we look at the relation below, + * as the trigger can run commands that change it, such as its + * persistence, and we must not undo those changes. + * + * We don't support Event Trigger for nested commands anywhere, + * here included, and parsetree is given NULL when coming from + * AlterTableInternal. + * + * And fire it only once. + */ + if (parsetree) + EventTriggerTableRewrite((Node *) parsetree, + tab->relid, + tab->rewrite); + OldHeap = table_open(tab->relid, NoLock); /* @@ -6033,21 +6050,6 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode, table_close(OldHeap, NoLock); - /* - * Fire off an Event Trigger now, before actually rewriting the - * table. - * - * We don't support Event Trigger for nested commands anywhere, - * here included, and parsetree is given NULL when coming from - * AlterTableInternal. - * - * And fire it only once. - */ - if (parsetree) - EventTriggerTableRewrite((Node *) parsetree, - tab->relid, - tab->rewrite); - /* * Create transient table that will receive the modified data. * diff --git a/src/test/regress/expected/event_trigger.out b/src/test/regress/expected/event_trigger.out index c947f56602a..f1f8840b820 100644 --- a/src/test/regress/expected/event_trigger.out +++ b/src/test/regress/expected/event_trigger.out @@ -681,6 +681,29 @@ ERROR: could not change table "rewrite_ufk" to logged because it references unl DROP EVENT TRIGGER rewrite_notice; DROP FUNCTION test_evtrig_rewrite_notice(); DROP TABLE rewrite_ufk, rewrite_upk; +-- changes made by a table_rewrite event trigger to the table being rewritten +-- are not undone by the rewrite +CREATE UNLOGGED TABLE rewrite_persist (a int); +CREATE FUNCTION test_evtrig_set_logged() RETURNS event_trigger +LANGUAGE plpgsql AS $$ +BEGIN + IF (pg_event_trigger_table_rewrite_reason() & 1) = 0 THEN + ALTER TABLE rewrite_persist SET LOGGED; + END IF; +END; +$$; +CREATE EVENT TRIGGER set_logged ON table_rewrite + EXECUTE FUNCTION test_evtrig_set_logged(); +ALTER TABLE rewrite_persist ALTER a TYPE bigint; +SELECT relpersistence FROM pg_class WHERE oid = 'rewrite_persist'::regclass; + relpersistence +---------------- + p +(1 row) + +DROP EVENT TRIGGER set_logged; +DROP FUNCTION test_evtrig_set_logged(); +DROP TABLE rewrite_persist; -- Tests for REINDEX CREATE OR REPLACE FUNCTION reindex_start_command() RETURNS event_trigger AS $$ diff --git a/src/test/regress/sql/event_trigger.sql b/src/test/regress/sql/event_trigger.sql index 3167d6d30f0..0f24c6ae97e 100644 --- a/src/test/regress/sql/event_trigger.sql +++ b/src/test/regress/sql/event_trigger.sql @@ -517,6 +517,25 @@ DROP EVENT TRIGGER rewrite_notice; DROP FUNCTION test_evtrig_rewrite_notice(); DROP TABLE rewrite_ufk, rewrite_upk; +-- changes made by a table_rewrite event trigger to the table being rewritten +-- are not undone by the rewrite +CREATE UNLOGGED TABLE rewrite_persist (a int); +CREATE FUNCTION test_evtrig_set_logged() RETURNS event_trigger +LANGUAGE plpgsql AS $$ +BEGIN + IF (pg_event_trigger_table_rewrite_reason() & 1) = 0 THEN + ALTER TABLE rewrite_persist SET LOGGED; + END IF; +END; +$$; +CREATE EVENT TRIGGER set_logged ON table_rewrite + EXECUTE FUNCTION test_evtrig_set_logged(); +ALTER TABLE rewrite_persist ALTER a TYPE bigint; +SELECT relpersistence FROM pg_class WHERE oid = 'rewrite_persist'::regclass; +DROP EVENT TRIGGER set_logged; +DROP FUNCTION test_evtrig_set_logged(); +DROP TABLE rewrite_persist; + -- Tests for REINDEX CREATE OR REPLACE FUNCTION reindex_start_command() RETURNS event_trigger AS $$ -- 2.56.0