Re: Assertion with aborted UPDATE in subtransaction

Hi,

I'd like to bring this thread back up. I ran into the same assertion
independently, and it still fails on master (4848fd6752a).

Why it matters to me: I develop an extension (pg_tviews) that keeps
summary tables up to date from statement-level AFTER triggers with
transition tables. The triggers upsert with INSERT ... ON CONFLICT DO
UPDATE, and some run inside PL/pgSQL exception blocks that roll back an
earlier update of the same row. I run my randomized correctness tests
on assert-enabled builds (16, 18 and 19), and this assertion crashes
the backend there. In practice I can't run that suite on cassert
builds until it's fixed. As Luc found, I saw no wrong results on
non-assert builds.

Here is a smaller repro than the original script (assert-enabled build,
fresh database):

CREATE TABLE t (a int, b text);
INSERT INTO t VALUES (1, '');
BEGIN;
SELECT * FROM t FOR NO KEY UPDATE;
SAVEPOINT s;
UPDATE t SET a = 2;
ROLLBACK TO SAVEPOINT s;
RELEASE SAVEPOINT s;
UPDATE t SET b = repeat('x', 3000); -- TRAP
COMMIT;

TRAP: failed Assert("HEAP_XMAX_IS_LOCKED_ONLY(infomask_lock_old_tuple)"),
File: "heapam.c", Line: 3893

It fails on master, REL_19_STABLE, 18.6 and REL_14_STABLE. The original
assert.sql no longer triggers it on REL_19 for me, because the page
never fills up, so the repro above uses a value over the TOAST threshold
instead.

Jasper's multixact.patch still applies cleanly to master, and with it:

- the repro above and the variants I tried pass (FOR UPDATE with a
key-changing update, ON CONFLICT DO UPDATE, MERGE, PL/pgSQL exception
blocks, a full page);
- make check and the isolation tests pass;
- on Luc's cost question: rebuilding a multixact with the same members
returns the existing one from the backend-local cache, so no extra
multixact IDs are used. I ran 500 TOAST updates while another
transaction held FOR KEY SHARE on the row, and both the patched and
unpatched builds used 2 multixacts. What remains is one
TransactionIdIsInProgress() call per member, and only on this path,
which as far as I can tell is reachable only from heap_update()'s
TOAST/new-page branch.

The patch still needs its comments updated: the function header and the
comment above the loop both still say the existing multixact is
returned as-is.

Attached is a regression test for combocid.sql that goes on top of
Jasper's patch. It has three cases, and each one crashes an
assert-enabled build without the fix.

In full disclosure: I'm not a PostgreSQL internals developer. The
analysis and the test were done with an AI assistant (Claude). I checked
the results by running them, but I can't vouch for the reasoning in
depth, so please treat it accordingly. I hope that, together with
Jasper's patch, this is enough to get the fix in.

Regards,
Lionel Hamayon