Re: table_rewrite event trigger can corrupt rows by inserting into the table being rewritten (20devel)

On Tue, 8 Sept 2026 at 15:38, Luguoqing <njuptlgq(at)163(dot)com> wrote:

> Sorry, the patch attachment in my previous email was missing or invalid.
> Please find the correct patch attached to this email.
>
>
> 2026年9月8日 18:12，路国庆 <njuptlgq(at)163(dot)com> 写道：
>
> Hi,
>
> I can reproduce a tuple corruption issue on PostgreSQL 20devel when a
> table_rewrite event trigger inserts into the table being rewritten by
> ALTER TABLE ... ALTER COLUMN ... TYPE.
>
> Environment:
> - PostgreSQL 20devel, commit 1a531f787f8
> (pg_resetwal: Add test for -o with negative value)
> - aarch64-apple-darwin24.6.0, 64-bit
> - Apple clang 15.0.0 (clang-1500.1.0.2.5)
>
> Minimal reproducer (please use a disposable database):
>
> CREATE TABLE trew_min(a int, b text);
>
> CREATE FUNCTION ev_min() RETURNS event_trigger LANGUAGE plpgsql AS $$
> BEGIN
> INSERT INTO trew_min VALUES (999, 'rw');
> END;
> $$;
>
> CREATE EVENT TRIGGER ev_min_trigger
> ON table_rewrite EXECUTE FUNCTION ev_min();
>
> ALTER TABLE trew_min ALTER COLUMN a TYPE bigint;
> SELECT a FROM trew_min;
> SELECT b FROM trew_min;
>
> The ALTER succeeds, and SELECT a returns 999. SELECT b then fails with:
>
> ERROR: XX000: invalid memory alloc request size 18446744073709551613
> LOCATION: MemoryContextSizeFailure, mcxt.c:1224
>
> Expected behavior would be either a valid rewritten row or rejection of
> unsafe access from the event trigger, with the ALTER rolled back. The ALTER
> should not succeed while leaving a malformed tuple behind.
>
> Looking at the code, ATExecAlterColumnType() updates pg_attribute during
> phase 2. By the time ATRewriteTables() calls EventTriggerTableRewrite(),
> those catalog changes are visible, but the heap still has its old layout.
> The trigger's INSERT therefore forms a tuple using (bigint, text), whereas
> ATRewriteTable() subsequently deforms it using tab->oldDesc, i.e. (int,
> text). On this little-endian machine the low four bytes still yield 999,
> but the high four bytes are interpreted as the following text datum's
> header. This explains why the first column appears correct while the
> second column is malformed. Reads during the same interval can also use
> the wrong tuple descriptor, so restricting INSERT alone seems insufficient.
>
> Attached is 0001-guard-table-rewrite-trigger-access.patch, an initial RFC
> patch, not a claim that this is the final API
> or the narrowest possible restriction. It rejects relation opens for the
> ALTER work queue while table_rewrite event triggers are running. It checks
> both relation_open() and try_relation_open(), covers other affected tables
> such as inheritance children, and stacks the state across nested rewrites.
> PG_FINALLY restores the previous state on both success and error, including
> errors caught by a PL/pgSQL exception handler. Catalog queries and writes
> to an unrelated audit table continue to work.
>
> The proposed guard deliberately covers the entire work queue, including
> relations already rewritten by the same command, rather than trying to
> expose partially completed ALTER state. It also rejects metadata helpers
> that open those relations. Whether that compatibility tradeoff is
> acceptable, and whether relation_open() is the right layer for the check,
> would benefit from review. I have not audited extension code that bypasses
> these relation-opening APIs or measured the added call overhead.
>
> Validation:
> - Built in a separate source/install directory with --without-icu,
> --enable-depend and --enable-cassert.
> - All 241 core regression tests passed, including the new test.
> - The new test covers INSERT, SELECT, prepared statements, UPDATE, DELETE,
> server-side COPY, TRUNCATE, inherited alterations, nested rewrites,
> exception recovery, and successful catalog/audit-table access.
> - Original behavior was also checked on an unpatched build of the same
> commit; see the attached reproduction output.
>
> I have not tested released branches or bisected the introduction of the
> problem. Is rejecting access in this interval the preferred approach, or
> should table_rewrite triggers run at a different point in ALTER processing?
>
> Regards
> 从 网易邮箱大师 发来的云附件
>
> <https://dashi.163.com/html/cloud-attachment-download/?key=djAydWdvUk5ETGFlT3VTNDBCemM0R3QxUT09>
> 0001-guard-table-rewrite-trigger-access.patch
> <https://dashi.163.com/html/cloud-attachment-download/?key=djAydWdvUk5ETGFlT3VTNDBCemM0R3QxUT09>
> 19.8K · 存在有效期
> 下载
> <https://dashi.163.com/html/cloud-attachment-download/?key=djAydWdvUk5ETGFlT3VTNDBCemM0R3QxUT09>
>
>
>

Hi! I reproduced this issue and confirmed it in 14-19 branches too.

First of all, people in these lists usually send patches as an attachment,
so you can read this from an archive many years later. Attaching your v1 in
my mail as-is

I did a quick look for this patch, and at first glance I'm not very happy
with the amount of work it adds for every command processing inside the
rewrite trigger. I didn;t measure the perf impact though. PG_TRY-PG_RESTORE
is also not free, a solution which does use this would be preferable (if
possible).

I also don;t think test coverage like this is actually needed:

+PREPARE rewrite_insert AS INSERT INTO rewrite_target VALUES (999, 'cached');
+SET regress.rewrite_command = 'EXECUTE rewrite_insert';
+ALTER TABLE rewrite_target ALTER COLUMN a TYPE bigint;
+ERROR: cannot access relation "rewrite_target" during a
table_rewrite event trigger
+DETAIL: The relation is being altered by the command that fired the
event trigger.
+QUERY: EXECUTE rewrite_insert
+CONTEXT: PL/pgSQL function rewrite_access() line 3 at EXECUTE
+DEALLOCATE rewrite_insert;

We can simply check single relation case and three-level inheritance, this
will actually cover most of what needs to be covered. 5 regress test
with regress.rewrite_command
is too much.

--
Best regards,
Kirill Reshke