From: Masahiko Sawada Date: Tue, 28 Jul 2026 19:33:28 +0000 (-0700) Subject: Fix logical decoding of empty prepared transactions. X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=acaa100f9a0c8dbd20ef48b3ce8b94bb773d6acc;p=thirdparty%2Fpostgresql.git Fix logical decoding of empty prepared transactions. A two-phase transaction that is assigned an XID but produces no change to be decoded -- for example, one that only acquires row locks via SELECT ... FOR SHARE -- has no base snapshot in the reorder buffer. ReorderBufferReplay() already skips such a transaction at PREPARE time and never invokes the begin_prepare/change/prepare callbacks for it, but ReorderBufferFinishPrepared() still called the commit_prepared (or rollback_prepared) callback. As a result a spurious COMMIT/ROLLBACK PREPARED was sent to the output plugin with no preceding PREPARE. For the built-in subscriber this breaks replication (the apply worker fails to find the prepared transaction), and test_decoding could even crash. Fix this by detecting an empty transaction (base_snapshot == NULL) in ReorderBufferFinishPrepared() and cleaning it up without invoking the commit/rollback prepared callbacks, mirroring the existing empty transaction handling in ReorderBufferReplay(). On v18 and newer versions, commit 072ee847ad4 changed ReorderBufferPrepare() to send the prepare whenever it had not already been sent, which also fires for empty transactions and emits a spurious PREPARE. On those branches ReorderBufferPrepare() is therefore additionally guarded with base_snapshot != NULL. This guard and the Assert(!rbtxn_sent_prepare()) added in ReorderBufferFinishPrepared(), are not necessary on v17 and older versions: there ReorderBufferPrepare() only sends a prepare for concurrently-aborted transactions (which never applies to an empty transaction) and the RBTXN_SENT_PREPARE flag does not exist. Back-patch to v14, where decoding of two-phase transactions was introduced. Bug: #19556 Reported-by: Alexander Kozhemyakin Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/19556-daa6d7ea65054d48@postgresql.org Backpatch-through: 14 --- diff --git a/contrib/test_decoding/expected/twophase.out b/contrib/test_decoding/expected/twophase.out index 08a7c56b5df..ea3c51f8215 100644 --- a/contrib/test_decoding/expected/twophase.out +++ b/contrib/test_decoding/expected/twophase.out @@ -227,6 +227,31 @@ SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'inc COMMIT PREPARED 'test_toast_table_access' (1 row) +-- Test that an empty prepared transaction should not be decoded, whether it +-- is committed or rolled back. +BEGIN; +SELECT * FROM test_prepared1 WHERE id = 1 FOR SHARE; + id | data +----+------ + 1 | +(1 row) + +PREPARE TRANSACTION 'test_empty_transaction'; +COMMIT PREPARED 'test_empty_transaction'; +BEGIN; +SELECT * FROM test_prepared1 WHERE id = 1 FOR SHARE; + id | data +----+------ + 1 | +(1 row) + +PREPARE TRANSACTION 'test_empty_transaction'; +ROLLBACK PREPARED 'test_empty_transaction'; +SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); + data +------ +(0 rows) + -- Test 8: -- cleanup and make sure results are also empty DROP TABLE test_prepared1; diff --git a/contrib/test_decoding/sql/twophase.sql b/contrib/test_decoding/sql/twophase.sql index 4b9ef0c0c44..834e5282c30 100644 --- a/contrib/test_decoding/sql/twophase.sql +++ b/contrib/test_decoding/sql/twophase.sql @@ -125,6 +125,18 @@ COMMIT PREPARED 'test_toast_table_access'; -- consume commit prepared SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1', 'stream-changes', '1'); +-- Test that an empty prepared transaction should not be decoded, whether it +-- is committed or rolled back. +BEGIN; +SELECT * FROM test_prepared1 WHERE id = 1 FOR SHARE; +PREPARE TRANSACTION 'test_empty_transaction'; +COMMIT PREPARED 'test_empty_transaction'; +BEGIN; +SELECT * FROM test_prepared1 WHERE id = 1 FOR SHARE; +PREPARE TRANSACTION 'test_empty_transaction'; +ROLLBACK PREPARED 'test_empty_transaction'; +SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); + -- Test 8: -- cleanup and make sure results are also empty DROP TABLE test_prepared1; diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c index 6df6166d8a7..6aed6346366 100644 --- a/src/backend/replication/logical/reorderbuffer.c +++ b/src/backend/replication/logical/reorderbuffer.c @@ -2979,11 +2979,18 @@ ReorderBufferPrepare(ReorderBuffer *rb, TransactionId xid, txn->prepare_time, txn->origin_id, txn->origin_lsn); /* - * Send a prepare if not already done so. This might occur if we have - * detected a concurrent abort while replaying the non-streaming - * transaction. + * Send a prepare if not already done so. The "not already sent" case can + * occur if we have detected a concurrent abort while replaying the + * non-streaming transaction; we still send the prepare so that later when + * rollback prepared is decoded and sent, the downstream should be able to + * rollback such a xact. See comments atop DecodePrepare. + * + * Skip this for a transaction that made no changes to the database (i.e. + * has no base snapshot), as we haven't sent any changes for it. Such a + * transaction is cleaned up without invoking the commit/rollback prepared + * callbacks in ReorderBufferFinishPrepared(). */ - if (!rbtxn_sent_prepare(txn)) + if (!rbtxn_sent_prepare(txn) && txn->base_snapshot != NULL) { rb->prepare(rb, txn, txn->final_lsn); txn->txn_flags |= RBTXN_SENT_PREPARE; @@ -3050,6 +3057,25 @@ ReorderBufferFinishPrepared(ReorderBuffer *rb, TransactionId xid, txn->prepare_time, txn->origin_id, txn->origin_lsn); } + /* + * If this transaction has no snapshot, it didn't make any changes to the + * database, so there's nothing to decode. Note that + * ReorderBufferCommitChild will have transferred any snapshots from + * subtransactions if there were any. + */ + if (txn->base_snapshot == NULL) + { + Assert(txn->ninvalidations == 0); + Assert(!rbtxn_sent_prepare(txn)); + + /* + * Removing this txn before a commit might result in the computation + * of an incorrect restart_lsn. See SnapBuildProcessRunningXacts. + */ + ReorderBufferCleanupTXN(rb, txn); + return; + } + txn->final_lsn = commit_lsn; txn->end_lsn = end_lsn; txn->commit_time = commit_time; diff --git a/src/test/subscription/t/021_twophase.pl b/src/test/subscription/t/021_twophase.pl index 4404d7b5449..f755e748e9c 100644 --- a/src/test/subscription/t/021_twophase.pl +++ b/src/test/subscription/t/021_twophase.pl @@ -309,6 +309,47 @@ $result = $node_subscriber->safe_psql('postgres', "SELECT count(*) FROM pg_prepared_xacts;"); is($result, qq(0), 'transaction is aborted on subscriber'); +############################### +# Test that an empty prepared transaction is not replicated. +# +# A transaction that is assigned an XID but makes no change decoded by logical +# replication (here, via a row lock) must not be sent to the subscriber. +# Otherwise the subscriber would receive a PREPARE with no preceding BEGIN +# PREPARE and error out, breaking replication. +############################### + +# An empty prepared transaction that is committed. +$node_publisher->safe_psql( + 'postgres', " + BEGIN; + SELECT a FROM tab_full WHERE a = 1 FOR SHARE; + PREPARE TRANSACTION 'test_empty_prepared'; + COMMIT PREPARED 'test_empty_prepared';"); + +# An empty prepared transaction that is rolled back. +$node_publisher->safe_psql( + 'postgres', " + BEGIN; + SELECT a FROM tab_full WHERE a = 1 FOR SHARE; + PREPARE TRANSACTION 'test_empty_prepared'; + ROLLBACK PREPARED 'test_empty_prepared';"); + +# A subsequent normal change must still replicate. Reaching catchup confirms +# the apply worker was not stalled by the empty prepared transactions above. +$node_publisher->safe_psql('postgres', "INSERT INTO tab_full VALUES (31);"); +$node_publisher->wait_for_catchup($appname); + +# The empty transactions must not have been prepared on the subscriber. +$result = $node_subscriber->safe_psql('postgres', + "SELECT count(*) FROM pg_prepared_xacts;"); +is($result, qq(0), 'empty prepared transaction is not replicated'); + +# The subsequent change is visible, so replication is healthy. +$result = $node_subscriber->safe_psql('postgres', + "SELECT count(*) FROM tab_full WHERE a = 31;"); +is($result, qq(1), + 'replication continues after an empty prepared transaction'); + ############################### # copy_data=false and two_phase ###############################