From 7127bdeed15c0de8af32e452c546b48e632c25ac Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Wed, 11 Nov 2026 22:57:03 +0000 Subject: [PATCH REL_17 v3 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 4138b86f771..11096b54fe8 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -5784,6 +5784,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); /* @@ -5841,21 +5858,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 8fe8b89280d..c8d38423fd8 100644 --- a/src/test/regress/expected/event_trigger.out +++ b/src/test/regress/expected/event_trigger.out @@ -655,6 +655,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 53e8b2c342c..279d0045c48 100644 --- a/src/test/regress/sql/event_trigger.sql +++ b/src/test/regress/sql/event_trigger.sql @@ -496,6 +496,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