20240612_PS_nitpicks_for_v4.txt

text/plain

Filename: 20240612_PS_nitpicks_for_v4.txt
Type: text/plain
Part: 0
Message: Re: Skip collecting decoded changes of already-aborted transactions
diff --git a/contrib/test_decoding/sql/stats.sql b/contrib/test_decoding/sql/stats.sql
index 7e05f39..a6a441d 100644
--- a/contrib/test_decoding/sql/stats.sql
+++ b/contrib/test_decoding/sql/stats.sql
@@ -54,14 +54,15 @@ COMMIT;
 SELECT 'init' FROM pg_create_logical_replication_slot('regression_slot_stats4_twophase', 'test_decoding', false, true) s4;
 
 -- Execute a transaction that is prepared and aborted. We detect that the
--- transaction is aborted before spilling changes, and skip to collect
--- further changes. So the transaction should not be spilled at all.
+-- transaction is aborted before spilling changes, and then skip collecting
+-- further changes. So, the transaction should not be spilled at all.
 BEGIN;
 INSERT INTO stats_test SELECT 'serialize-topbig--1:'||g.i FROM generate_series(1, 5000) g(i);
 TRUNCATE table stats_test;
 PREPARE TRANSACTION 'test1_abort';
 ROLLBACK PREPARED 'test1_abort';
--- should show only ROLLBACK PREAPRED.
+
+-- Should show only ROLLBACK PREPARED.
 SELECT data FROM pg_logical_slot_get_changes('regression_slot_stats4_twophase', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
 
 -- Check stats. We should not spill anything as the transaction is already
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 861ee1c..8b3e1b8 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -2802,7 +2802,7 @@ ReorderBufferPrepare(ReorderBuffer *rb, TransactionId xid,
 	txn->gid = pstrdup(gid);
 
 	/*
-	 * We remember whether the transaction is already aborted before the
+	 * Remember whether the transaction is already aborted before the
 	 * replay in order to detect the concurrent abort below.
 	 */
 	txn_aborted = rbtxn_did_abort(txn);
@@ -3608,18 +3608,20 @@ ReorderBufferLargestStreamableTopTXN(ReorderBuffer *rb)
 
 /*
  * Check the transaction status of the given transaction. If the transaction
- * already aborted, we discards all changes accumulated so far and ignore
- * future changes, and return true. Otherwise return false.
+ * already aborted, we discard all changes accumulated so far, ignore future
+ * changes, and return true. Otherwise return false.
  *
- * If logical_replication_mode is set to "immediate", we disable this check
- * for regression tests.
+ * If GUC 'debug_logical_replication_streaming' is "immediate", we don't
+ * check the transaction status, so the caller always processes this
+ * transaction. This is to disable this check for regression tests.
  */
 static bool
 ReorderBufferCheckTXNAbort(ReorderBuffer *rb, ReorderBufferTXN *txn)
 {
 	/*
-	 * If logical_replication_mode is "immediate", we don't check the
-	 * transaction status so the caller always processes this transaction.
+	 * If GUC 'debug_logical_replication_streaming' is "immediate", we don't
+	 * check the transaction status, so the caller always processes this
+	 * transaction.
 	 */
 	if (debug_logical_replication_streaming == DEBUG_LOGICAL_REP_STREAMING_IMMEDIATE)
 		return false;
diff --git a/src/include/replication/reorderbuffer.h b/src/include/replication/reorderbuffer.h
index b0d381c..8eb0704 100644
--- a/src/include/replication/reorderbuffer.h
+++ b/src/include/replication/reorderbuffer.h
@@ -227,19 +227,18 @@ typedef struct ReorderBufferChange
 	((txn)->txn_flags & RBTXN_PREPARE) != 0 \
 )
 
-/* Did this transaction committed? */
+/* Is this transaction committed? */
 #define rbtxn_did_commit(txn) \
 ( \
 	((txn)->txn_flags & RBTXN_COMMITTED) != 0 \
 )
 
-/* Did this transaction aborted? */
+/* Is this transaction aborted? */
 #define rbtxn_did_abort(txn) \
 ( \
 	((txn)->txn_flags & RBTXN_ABORTED) != 0 \
 )
 
-
 /* prepare for this transaction skipped? */
 #define rbtxn_skip_prepared(txn) \
 ( \