From e71735ee759d49386810c098303255accad1c8b4 Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Wed, 11 Nov 2026 22:57:03 +0000 Subject: [PATCH REL_17 v4 3/3] 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 40e7b462110..0339fe35beb 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -5800,6 +5800,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); /* @@ -5857,21 +5874,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 16f0ccf8541..f24bbb086e7 100644 --- a/src/test/regress/expected/event_trigger.out +++ b/src/test/regress/expected/event_trigger.out @@ -671,6 +671,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 117b2987729..5b5627e27d6 100644 --- a/src/test/regress/sql/event_trigger.sql +++ b/src/test/regress/sql/event_trigger.sql @@ -512,6 +512,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