0007-Edits-on-0002-patch-20230720.patch

From 9be6a6be004f71c80b2f88fd459207866ade061e Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Date: Tue, 18 Jul 2023 19:24:17 +0530
Subject: [PATCH 7/7] Edits on 0002 patch

---
 src/backend/replication/logical/decode.c      |  2 +-
 src/backend/replication/logical/logical.c     |  8 +--
 .../replication/logical/reorderbuffer.c       | 56 +++++++++----------
 3 files changed, 32 insertions(+), 34 deletions(-)

diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c
index e08052617d..e24ccd9af6 100644
--- a/src/backend/replication/logical/decode.c
+++ b/src/backend/replication/logical/decode.c
@@ -1412,7 +1412,7 @@ sequence_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
 	/*
 	 * Should we handle the sequence increment as transactional or not?
 	 *
-	 * If the sequence was created in a still-running transaction, treat
+	 * If the sequence was assigned a new relfilenode in the transaction being decoded, treat
 	 * it as transactional and queue the increments. Otherwise it needs
 	 * to be treated as non-transactional, in which case we send it to
 	 * the plugin right away.
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index d2ff674e32..74ba6fd861 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -231,10 +231,10 @@ StartupDecodingContext(List *output_plugin_options,
 
 	/*
 	 * To support streaming, we require start/stop/abort/commit/change
-	 * callbacks. The message and truncate callbacks are optional, similar to
-	 * regular output plugins. We however enable streaming when at least one
-	 * of the methods is enabled so that we can easily identify missing
-	 * methods.
+	 * callbacks. The message, sequence and truncate callbacks are optional,
+	 * similar to regular output plugins. We however enable streaming when at
+	 * least one of the methods is enabled so that we can easily identify
+	 * missing methods.
 	 *
 	 * We decide it here, but only check it later in the wrappers.
 	 */
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index d0bcc5a2f8..6f5678fd2f 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -77,36 +77,33 @@
  *	  a bit more memory to the oldest subtransactions, because it's likely
  *	  they are the source for the next sequence of changes.
  *
- *	  When decoding sequences, we differentiate between a sequences created
- *	  in a (running) transaction, and sequences created in other (already
- *	  committed) transactions. Changes for sequences created in the same
- *	  top-level transaction are treated as "transactional" i.e. just like
- *	  any other change from that transaction (and discarded in case of a
- *	  rollback). Changes for sequences created earlier are treated as not
- *	  transactional - are processed immediately, as if performed outside
- *	  any transaction (and thus not rolled back).
- *
+ *    When decoding sequences, we differentiate between changes to the
+ *    sequences assigned a new relfilenode in in a transaction being decoded,
+ *    and the changes to the sequences created in other (already committed)
+ *    transactions. First type of changes are treated as "transactional" i.e.
+ *    just like any other change from that transaction (and discarded in case
+ *    of a rollback). Second type of changes are treated as non-transactional.
+ *    They are processed immediately, as if performed outside any transaction
+ *    (and thus not rolled back).
+ *   
  *	  This mixed behavior is necessary - sequences are non-transactional
  *	  (e.g. ROLLBACK does not undo the sequence increments). But for new
  *	  sequences, we need to handle them in a transactional way, because if
  *	  we ever get some DDL support, the sequence won't exist until the
- *	  transaction gets applied. So we need to ensure the increments don't
- *	  happen until the sequence gets created.
- *
- *	  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 relfilelocator,
- *	  and we track XID of the (sub)transaction that created it. This means
- *	  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).
- *
- *	  We don't use the XID to check if it's the same top-level transaction.
- *	  It's enough to know it was created in an in-progress transaction,
- *	  and we know it must be the current one because otherwise it wouldn't
- *	  see the sequence object.
+ *	  transaction gets applied. So we need to ensure the changes don't
+ *	  happen until the sequence gets created. Similarly when a new relfilenode is assigned to a sequence (because of a DDL) the change does not become visible till the transaction gets committed. It the transaction gets aborted it will never be visible.
  *
+ *    To differentiate between the changes to the sequences, we track sequences
+ *    which are assigned a new relfilenode in transactions being decoded in a
+ *    hash table.  We also track XID of the (sub)transaction that assigned the
+ *    new relfilenode. The list of sequence relfilenodes gets discarded when
+ *    the transaction completes (commit/rollback).
+ *   
+ *    We don't use the XID to check if it's the same top-level transaction.
+ *    It's enough to know it was created in the transaction being decoded, and
+ *    we know it must be the current one because otherwise it wouldn't see the
+ *    sequence object.
+ *   
  *	  The XID may be valid even for non-transactional sequences - we simply
  *	  keep the XID logged to WAL, it's up to the reorderbuffer to decide if
  *	  the increment is transactional.
@@ -953,11 +950,12 @@ ReorderBufferQueueMessage(ReorderBuffer *rb, TransactionId xid,
 }
 
 /*
- * Treat the sequence increment as transactional?
+ * Treat the sequence change as transactional?
  *
- * The hash table tracks all sequences created in in-progress transactions,
- * so we simply do a lookup (the sequence is identified by relfilende). If
- * we find a match, the increment should be handled as transactional.
+ * The hash table tracks all sequences which are assigned a new relfilenode in
+ * the transaction being decoded, so we simply do a lookup (the sequence is
+ * identified by relfilende). If we find a match, the change should be handled
+ * as transactional.
  */
 bool
 ReorderBufferSequenceIsTransactional(ReorderBuffer *rb,
-- 
2.25.1