Re: Bug in logical decoding with DDL and subtransactions

On 4/30/2026 12:59 PM, Mark Dilger wrote:
> There is a bug in logical decoding with CREATE and subtransactions. 
> If a CREATE statement creates a row in a catalog during a
> subtransaction, but that subtransaction gets rolled back to the
> savepoint, and other things happen which trigger page pruning on the
> catalog page, and the original transaction (perhaps in a new
> subtransaction) then does another CREATE operation, a new row can get
> inserted into the same catalog at the same TID.
Hi Mark,

Bug #19555 [1] appears to be another instance of this issue: an
assertion failure in ReorderBufferBuildTupleCidHash
("ent->cmin == change->data.tuplecid.cmin") when DDL runs inside a
savepoint that is then rolled back.  I reproduced it on REL_19_STABLE
(64542957b44) using the script from the bug report, and on current
master (36f7330b8b2) with a slightly different table shape:

    CREATE TABLE zz(data int, pad1 text, pad2 int, pad3 text);
    BEGIN;
    SAVEPOINT a;
    ALTER TABLE zz ALTER COLUMN data TYPE text;
    ROLLBACK TO SAVEPOINT a;
    ALTER TABLE zz ALTER COLUMN data TYPE bigint;
    COMMIT;
    SELECT data FROM pg_logical_slot_get_changes('regression_slot',
        NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');

The crash is layout-dependent: it needs on-access pruning to free the
aborted subtransaction's line pointer and a later catalog insert to
reuse it, so the catalog churn from the bug report's prelude matters.
With the 4-column table above it crashed on the first attempt.

I traced the WAL and can confirm your diagnosis:

- the rolled-back subtransaction's NEW_CID records stay queued under
  the top-level xid, and ReorderBufferAbort() never removes them;
- its HOT-updated catalog tuple becomes a dead heap-only tuple, which
  on-access pruning marks LP_UNUSED (no vacuum needed);
- the next ALTER's catalog insert reuses that TID, producing a second
  NEW_CID for the same (relfilelocator, tid) with a different cmin.

With your patch applied to master, the previously-crashing case passes
(20 iterations, no crash, no "cmin mismatch" warning), and the
test_decoding regression suite passes.

A few review comments:

1. The patch fixes the crashing path, but the mirror case remains: a
   catalog tuple *deleted* only by an aborted subtransaction keeps a
   stale cmax in the hash, so historic snapshots could wrongly treat a
   live tuple as deleted.  Did you consider removing the aborted
   subtransaction's tuplecid entries at ReorderBufferAbort() time,
   instead of resolving conflicts lazily at hash-build time?  That
   would cover both cases and avoid the TransactionIdDidAbort() clog
   lookup during decode.  I'm happy to write that variant if you think
   it is worth comparing.

2. For the "neither subtransaction aborted" branch, would an Assert
   be better than elog(WARNING)?  A warning from the decoding path
   seems odd, and if the case is truly impossible an assertion would
   document that.

3. The patch has no regression test.  The scenario above is the
   smallest reproducer I know of, though its layout dependence makes
   it fragile for the test suite.  Any suggestions for making it
   deterministic?

It would be great to get this fixed for 19: the bug report shows it
is being hit by fuzzers in the wild, and the cmax half of this
problem was already fixed in 8c67d29fd51 back in 2019.

[1]
https://www.postgresql.org/message-id/19555-3c700f40a13045cd@postgresql.org

Thanks,
<Bingshuai Li>