Further cleanup of ReorderBufferCommit().

On closer inspection, we can remove the "volatile" qualifier on
"using_subtxn" so long as we initialize that before the PG_TRY block,
which there's no particularly good reason not to do.
Also, push the "change" variable inside the PG_TRY so as to remove
all question of whether it needs "volatile", and remove useless
early initializations of "snapshow_now" and "using_subtxn".

diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 82f83d1450516e488c2e8bbfabf3bcacc39bd673..bcd58966f0aca69858ec4eccf707bfbfb826d380 100644 (file)
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -1258,12 +1258,10 @@ ReorderBufferCommit(ReorderBuffer *rb, TransactionId xid,
                                        TimestampTz commit_time)
 {
        ReorderBufferTXN *txn;
-       ReorderBufferIterTXNState *volatile iterstate = NULL;
-       ReorderBufferChange *change;
-
+       volatile Snapshot snapshot_now;
        volatile CommandId command_id = FirstCommandId;
-       volatile Snapshot snapshot_now = NULL;
-       volatile bool using_subtxn = false;
+       bool            using_subtxn;
+       ReorderBufferIterTXNState *volatile iterstate = NULL;
 
        txn = ReorderBufferTXNByXid(rb, xid, false, NULL, InvalidXLogRecPtr,
                                                                false);
@@ -1301,19 +1299,21 @@ ReorderBufferCommit(ReorderBuffer *rb, TransactionId xid,
        /* setup the initial snapshot */
        SetupHistoricSnapshot(snapshot_now, txn->tuplecid_hash);
 
+       /*
+        * Decoding needs access to syscaches et al., which in turn use
+        * heavyweight locks and such. Thus we need to have enough state around to
+        * keep track of those.  The easiest way is to simply use a transaction
+        * internally.  That also allows us to easily enforce that nothing writes
+        * to the database by checking for xid assignments.
+        *
+        * When we're called via the SQL SRF there's already a transaction
+        * started, so start an explicit subtransaction there.
+        */
+       using_subtxn = IsTransactionOrTransactionBlock();
+
        PG_TRY();
        {
-               /*
-                * Decoding needs access to syscaches et al., which in turn use
-                * heavyweight locks and such. Thus we need to have enough state
-                * around to keep track of those. The easiest way is to simply use a
-                * transaction internally. That also allows us to easily enforce that
-                * nothing writes to the database by checking for xid assignments.
-                *
-                * When we're called via the SQL SRF there's already a transaction
-                * started, so start an explicit subtransaction there.
-                */
-               using_subtxn = IsTransactionOrTransactionBlock();
+               ReorderBufferChange *change;
 
                if (using_subtxn)
                        BeginInternalSubTransaction("replay");
@@ -1323,7 +1323,7 @@ ReorderBufferCommit(ReorderBuffer *rb, TransactionId xid,
                rb->begin(rb, txn);
 
                iterstate = ReorderBufferIterTXNInit(rb, txn);
-               while ((change = ReorderBufferIterTXNNext(rb, iterstate)))
+               while ((change = ReorderBufferIterTXNNext(rb, iterstate)) != NULL)
                {
                        Relation        relation = NULL;
                        Oid                     reloid;