0006-john-s-review-20230316.patch

From 27b4def5b0cdc3df8a1f32058e65021db6b9c06c Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas.vondra@postgresql.org>
Date: Thu, 16 Mar 2023 17:03:19 +0100
Subject: [PATCH 6/6] john's review

---
 src/backend/replication/logical/decode.c      |  2 +-
 .../replication/logical/reorderbuffer.c       | 20 ++++++++++---------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c
index aece336612e..cd2f88e0bde 100644
--- a/src/backend/replication/logical/decode.c
+++ b/src/backend/replication/logical/decode.c
@@ -1359,7 +1359,7 @@ sequence_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
 	/* extract the WAL record, with "created" flag */
 	xlrec = (xl_seq_rec *) XLogRecGetData(r);
 
-	/* XXX how could we have sequence change without data? */
+	/* the sequence should not have changed without data */
 	if(!datalen || !tupledata)
 		elog(ERROR, "sequence decode missing tuple data");
 
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 0c8cca3029c..16fcbed42c9 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -95,10 +95,10 @@
  *
  *	  To differentiate which sequences are "old" and which were created
  *	  in a still-running transaction, we track sequences created in running
- *	  transactions in a hash table. Sequences are identified by relfilenode,
+ *	  transactions in a hash table. Sequences are identified by relfilelocator,
  *	  and we track XID of the (sub)transaction that created it. This means
- *	  that if a transaction does something that changes the relfilenode
- *	  (like an alter / reset of a sequence), the new relfilenode will be
+ *	  that if a transaction does something that changes the relfilelocator
+ *	  (like an alter / reset of a sequence), the new relfilelocator will be
  *	  treated as if created in the transaction. The list of sequences gets
  *	  discarded when the transaction completes (commit/rollback).
  *
@@ -391,7 +391,7 @@ ReorderBufferAllocate(void)
 	buffer->by_txn = hash_create("ReorderBufferByXid", 1000, &hash_ctl,
 								 HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
 
-	/* hash table of sequences, mapping relfilenode to XID of transaction */
+	/* hash table of sequences, mapping relfilelocator to XID of transaction */
 	hash_ctl.keysize = sizeof(RelFileLocator);
 	hash_ctl.entrysize = sizeof(ReorderBufferSequenceEnt);
 	hash_ctl.hcxt = buffer->context;
@@ -1043,7 +1043,7 @@ ReorderBufferQueueSequence(ReorderBuffer *rb, TransactionId xid,
 		MemoryContext oldcontext;
 		ReorderBufferChange *change;
 
-		/* lookup sequence by relfilenode */
+		/* lookup sequence by relfilelocator */
 		ReorderBufferSequenceEnt   *ent;
 		bool						found;
 
@@ -1079,9 +1079,7 @@ ReorderBufferQueueSequence(ReorderBuffer *rb, TransactionId xid,
 		if (created)
 			ent->xid = xid;
 
-		/* XXX Maybe check that we're still in the same top-level xact? */
-
-		/* OK, allocate and queue the change */
+		/* allocate and queue the transactional sequence change */
 		oldcontext = MemoryContextSwitchTo(rb->context);
 
 		change = ReorderBufferGetChange(rb);
@@ -2319,7 +2317,11 @@ ReorderBufferApplySequence(ReorderBuffer *rb, ReorderBufferTXN *txn,
 	tuple = &change->data.sequence.tuple->tuple;
 	seq = (Form_pg_sequence_data) GETSTRUCT(tuple);
 
-	/* Only ever called from ReorderBufferApplySequence, so transational. */
+	/*
+	 * When called from ReorderBufferApplySequence, we're applying changes
+	 * accumulated in a ReorderBufferTXN, so all those are transactional
+	 * changes of sequences.
+	 */
 	if (streaming)
 		rb->stream_sequence(rb, txn, change->lsn, relation, true,
 							seq->last_value, seq->log_cnt, seq->is_called);
-- 
2.39.2