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 059ed860314..08be0dacc4a 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 ###############################