From 807fe068f4fcb4f9d3048b47fb491840e8325551 Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Wed, 11 Nov 2026 22:57:03 +0000 Subject: [PATCH v3 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 248fbafa2c3..1b0c9916188 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -5962,6 +5962,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); /* @@ -6019,21 +6036,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 d24d78450d0..06ed5eef464 100644 --- a/src/test/regress/expected/event_trigger.out +++ b/src/test/regress/expected/event_trigger.out @@ -665,6 +665,29 @@ ERROR: could not change table "rewrite_ppk" to unlogged because it references l DROP EVENT TRIGGER add_fk; DROP FUNCTION test_evtrig_add_fk(); DROP TABLE rewrite_ufk, rewrite_upk, rewrite_pfk, rewrite_ppk; +-- 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 ad82a548ea5..20dc7148d92 100644 --- a/src/test/regress/sql/event_trigger.sql +++ b/src/test/regress/sql/event_trigger.sql @@ -501,6 +501,25 @@ DROP EVENT TRIGGER add_fk; DROP FUNCTION test_evtrig_add_fk(); DROP TABLE rewrite_ufk, rewrite_upk, rewrite_pfk, rewrite_ppk; +-- 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