Bug in logical decoding with DDL and subtransactions

[pg-hub-mirror[bot]]

Read-only mirror. Join the conversation on pgsql-hackers; activity here is not sent upstream.

• Original author: Alexander Lakhin <exclusion(at)gmail(dot)com>
• Mailing list: pgsql-hackers
• Message-ID: 511b76ca-8aff-4f6a-b1e5-18d1467affb0@gmail.com
• Original email

---

Hello hackers,
27.08.2026 10:38, Bingshuai Li wrote:

A brief ping on v4 before PG20-2 starts. The latest CFBot run is
green on all enabled jobs:

https://github.com/postgres/postgres-cfbot/actions/runs/32910623105/attempts/1
....
PostgreSQL 19 is currently at Beta 3. Should BUG #19555 also be
tracked as a PostgreSQL 19 open item?

CF entry:
https://commitfest.postgresql.org/patch/7137/
FWIW, buildfarm animal prion produced a couple of failures recently due to
that assertion triggered, [1], [2]:
TRAP: failed Assert("ent->cmin == change->data.tuplecid.cmin"), File:
"/home/ec2-user/bf/root/HEAD/pgsql.build/../pgsql/src/backend/replication/logical/reorderbuffer.c", Line: 1890, PID: 1211632
...
2026-09-15 21:58:17.665 UTC [1211335:63] LOG:  00000: client backend (PID 1211632) was terminated by signal 6: Aborted
2026-09-15 21:58:17.665 UTC [1211335:64] DETAIL:  Failed process was running: SELECT data FROM
pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=prion&dt=2026-09-15%2021%3A39%3A57
[2] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=prion&dt=2026-09-15%2022%3A13%3A04
Best regards,
Alexander

[pg-hub-mirror[bot]]

Bingshuai Li <lucian1412(at)outlook(dot)com> via pgsql-hackers · original email

Hi Alexander,
Thanks for the data point. These failures are exactly the assertion
analyzed in this thread, and the failing statement is the decode call
right after the ddl test's own "DDL in aborted subtransactions"
section (contrib/test_decoding/sql/ddl.sql):
CREATE TABLE tr_sub_ddl(data int);
BEGIN;
SAVEPOINT a;
ALTER TABLE tr_sub_ddl ALTER COLUMN data TYPE text;
INSERT INTO tr_sub_ddl VALUES ('blah-blah');
ROLLBACK TO SAVEPOINT a;
ALTER TABLE tr_sub_ddl ALTER COLUMN data TYPE bigint;
INSERT INTO tr_sub_ddl VALUES(43);
COMMIT;
That is the same shape as the BUG #19555 reproducer, and the lines
missing from the ddl.out diff are everything after the backend died.
The failure is inherently layout-dependent: it needs on-access
pruning to free the rolled-back subtransaction's catalog line
pointer and a later catalog insert to reuse that tid. Presumably
prion's force-release configuration (RELCACHE_FORCE_RELEASE,
CATCACHE_FORCE_RELEASE) makes that far more likely than on other
animals -- two hits in one evening supports that.
The v4 patch posted on 2026-08-15 [1] fixes exactly this path: it
removes the aborted subtransaction's tuplecid entries in
ReorderBufferAbort(), so no stale (relfilelocator, tid) mapping
remains to collide with a reused tid. The regression test added by
v4 reproduces this crash within a handful of iterations on
unpatched assert builds and always passes with the patch.
I re-checked v4 against current master (be00f04) today: it
still applies cleanly, and no upstream commit has touched the
tuplecid code paths since it was posted, so the verification matrix
from the v4 mail (master and REL_19_STABLE, assert and non-assert,
both build systems) still stands.
With the buildfarm now red on HEAD and PostgreSQL 19 GA
approaching, could a committer please take a look at v4? The two
points where I would most appreciate judgement are unchanged from
my August 27 mail: the restart_lsn invariant (section 2 of the v4
mail), and whether the O(N) abort-time cleanup is acceptable to
backpatch. I also believe this strengthens the case for tracking
BUG #19555 as a PostgreSQL 19 open item. I'm happy to prepare a
REL_19_STABLE version of the patch if that would help.
[1] https://www.postgresql.org/message-id/SY8P300MB0570D533D4F6B47B804C7D57C6D92%40SY8P300MB0570.AUSP300.PROD.OUTLOOK.COM
Thanks,
Bingshuai Li

[pg-hub-mirror[bot]]

"alvherre(at)kurilemu(dot)de" <alvherre(at)kurilemu(dot)de> via pgsql-hackers · original email

On 2026-Sep-17, Bingshuai Li wrote:

With the buildfarm now red on HEAD and PostgreSQL 19 GA
approaching, could a committer please take a look at v4? The two
points where I would most appreciate judgement are unchanged from
my August 27 mail: the restart_lsn invariant (section 2 of the v4
mail), and whether the O(N) abort-time cleanup is acceptable to
backpatch. I also believe this strengthens the case for tracking
BUG #19555 as a PostgreSQL 19 open item.
What's the reason to consider this as a worse problem in 19 than in
previous releases? In my mind it's pretty clear that this bug needs to
be fixed in all stable branches. Moreover, I think the real deadline
here is the next set of minors to be released in November, rather than
the 19.0 release. The November minors would also include 19.1.
I'm happy to prepare a REL_19_STABLE version of the patch if that
would help.
If the proposed patch has any conflicts when applied on any previous
branches, then by all means please do provide backpatches as
appropriate. However, if the same patch applies cleanly or only has
trivial conflicts, then there's no need. (I see there's a trivial
conflict in 18's test schedule files for instance, for which IMO you
don't need to provide a patch. Didn't try further back.)
The patch does change the layout of ReorderBufferChange, which is
apparently an exported struct, so some analysis on whether that can
break anything would also be useful. It might be innocuous, if there
are no external users, but better make sure ...
--- /tmp/ReorderBufferChange.unpatched 2026-09-17 13:09:09.517276936 +0200
+++ /tmp/ReorderBufferChange.patched 2026-09-17 13:09:35.505580268 +0200
@@ -37,27 +37,29 @@
CommandId command_id; /* 32 4 /
struct {
RelFileLocator locator; / 32 12 /
ItemPointerData tid attribute((aligned(2))); / 44 6 /
/ XXX 2 bytes hole, try to pack /
CommandId cmin; / 52 4 /
CommandId cmax; / 56 4 /
CommandId combocid; / 60 4 */

•   } __attribute__((__aligned__(4))) tuplecid __attribute__((__aligned__(4)));  /*    32    32 */
  

•       /* --- cacheline 1 boundary (64 bytes) --- */
  

•       TransactionId subxid;            /*    64     4 */
  

•   } __attribute__((__aligned__(4))) tuplecid __attribute__((__aligned__(4)));  /*    32    36 */
    struct {
        uint32     ninvalidations;       /*    32     4 */
        /* XXX 4 bytes hole, try to pack */
        SharedInvalidationMessage * invalidations; /*    40     8 */
    } inval;                                 /*    32    16 */
  

• } data attribute((aligned(8))); /* 32 32 */
• /* --- cacheline 1 boundary (64 bytes) --- */
• dlist_node node; /* 64 16 */

• } data attribute((aligned(8))); /* 32 40 */
• /* --- cacheline 1 boundary (64 bytes) was 8 bytes ago --- */
• dlist_node node; /* 72 16 */

• /* size: 80, cachelines: 2, members: 6 */
• /* sum members: 70, holes: 2, sum holes: 10 */

• /* size: 88, cachelines: 2, members: 6 */
• /* sum members: 78, holes: 2, sum holes: 10 /
  / forced alignments: 1, forced holes: 1, sum forced holes: 6 */

• /* last cacheline: 16 bytes */

• /* last cacheline: 24 bytes */
  } attribute((aligned(8)));
  --
  Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
  "Debido a que la velocidad de la luz es mucho mayor que la del sonido,
  algunas personas nos parecen brillantes un minuto antes
  de escuchar las pelotudeces que dicen." (Roberto Fontanarrosa)

[pg-hub-mirror[bot]]

Bingshuai Li <lucian1412(at)outlook(dot)com> via pgsql-hackers · original email

Hi Álvaro,
Thanks for picking this up.

What's the reason to consider this as a worse problem in 19 than in
previous releases?
There isn't one -- I asked because 19 GA was approaching and the bug
was reported against 19. Fixing it in all stable branches for the
November minors sounds right.
If the proposed patch has any conflicts when applied on any previous
branches, then by all means please do provide backpatches as
appropriate.
I ran v4 across all supported branches (--enable-cassert build plus
the full test_decoding suite on each):
branch apply tests (regress+isolation)

---

master clean (be00f04) 21+15, CFBot green
19 clean (a203d20) 21+15
18 trivial test-list conflicts 20+14
17 branch version attached 20+14
16 branch version attached 20+14
15 branch version attached 20+14
14 branch version attached 20+13
(The counts follow each branch's existing test_decoding suite;
tuplecid and tuplecid_restart pass everywhere.)
On 18 the conflicts are exactly the trivial ones you saw: master's
test lists mention repack and parallel_session_origin, which don't
exist there. All code applies cleanly, so per your advice I have not
prepared an 18 version.
The attached branch versions differ from v4 as follows:

• 17, 16: only the test-list context, plus one rename -- the cleanup
  calls ReorderBufferReturnChange(), the pre-PG18 name of
  ReorderBufferFreeChange() (renamed by ac44946; same 3-argument
  signature and body). The 17 and 16 versions differ from each other
  only in the test-list context, too.
• 15, 14: additionally drop the meson.build hunk (meson arrived in
  16) and use the pre-PG16 names: RelFileNode, xlrec->target_node,
  and txn->toptxn instead of rbtxn_get_toptxn(). The 14 version also
  trims the new test's expected output, because 14's psql prints only
  the last result of a multi-statement string; the decode work is
  unchanged, so the test exercises the bug just the same.
  rbtxn_is_known_subxact() exists natively in all of these branches, so
  the fix itself is identical everywhere.

The patch does change the layout of ReorderBufferChange, which is
apparently an exported struct, so some analysis on whether that can
break anything would also be useful.
I checked who can observe the layout change:

• the tuplecid variant is only ever accessed from reorderbuffer.c
  itself (no data.tuplecid users anywhere else in tree);
• only reorderbuffer.c touches change->node, and only reorderbuffer.c
  allocates, frees or serializes ReorderBufferChange (the spill files
  embed the struct, but they are transient within a single decoding
  session);
• output plugins (pgoutput, pgrepack, test_decoding, and external
  ones using the same callback API) only read change->action, ->txn,
  ->data.tp.{old,new}tuple and ->data.truncate.*, all at offsets
  unchanged by the patch: the union only grows at the tail of the
  tuplecid variant, and the struct stays within two cachelines, as
  your pahole output shows.
  So the layout change is innocuous for any plugin I can think of.
  There is one more exported-API change next to it:
  ReorderBufferAddNewTupleCids() gains a subxid parameter. Its only
  caller is the NEW_CID decode path in snapbuild.c; output plugins have
  no reason to call it. If you'd rather keep the exported signature
  stable on the back branches, I can keep the old one as a wrapper
  passing InvalidTransactionId -- it is about five lines.
  The part of v4 where I would still most value a second opinion is the
  restart_lsn invariant in section 2 of the v4 mail (why the skipped
  cleanup on the restart path cannot reach the tuplecid hash); I believe
  everything else is covered above.
  Thanks,
  Bingshuai Li

[pg-hub-mirror[bot]]

A patch is now available as PR #418.