Re: Null commitTS bug - Mailing list pgsql-hackers

From Kyotaro Horiguchi
Subject Re: Null commitTS bug
Date
Msg-id 20220120.120056.82643992708364090.horikyota.ntt@gmail.com
Whole thread Raw
In response to Re: Null commitTS bug  (Kyotaro Horiguchi <horikyota.ntt@gmail.com>)
List pgsql-hackers
At Tue, 18 Jan 2022 13:48:11 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in 
> At Tue, 18 Jan 2022 10:43:55 +0900, Michael Paquier <michael@paquier.xyz> wrote in 
> > On Sun, Jan 16, 2022 at 11:01:25PM -0500, Tom Lane wrote:
> > > Isn't that a very bad way to write "i = j + 1"?
> > > 
> > > I agree with Horiguchi-san that
> > >     for (i = 0, headxid = xid;;)
> > 
> > Okay.  Horiguchi-san, would you like to write a patch?
> 
> Yes, I will.

This is that. I think this is a separate issue from the actual
bug. This is applicable at least back to 9.6 and I think this should
be applied back to all supported versions to avoid future backptach
conflicts.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From 9211fa61513dcdbca273656454395a3dcf3ee4e7 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Thu, 20 Jan 2022 10:16:48 +0900
Subject: [PATCH] Improve confusing code in TransactionTreeSetCommitTsData

TransactionTreeSetCommitTsData has a bit confusing use of the +=
operator.  Simplifying it makes the code easier to follow.  In the
same function for-loop initializes only non-controlling variables,
which is not great style.  They ought to be initialized outside the
loop.
---
 src/backend/access/transam/commit_ts.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 659109f8d4..88eac10456 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -168,7 +168,10 @@ TransactionTreeSetCommitTsData(TransactionId xid, int nsubxids,
      * subxid not on the previous page as head.  This way, we only have to
      * lock/modify each SLRU page once.
      */
-    for (i = 0, headxid = xid;;)
+    headxid = xid;
+    i = 0;
+
+    for (;;)
     {
         int            pageno = TransactionIdToCTsPage(headxid);
         int            j;
@@ -192,7 +195,7 @@ TransactionTreeSetCommitTsData(TransactionId xid, int nsubxids,
          * just wrote.
          */
         headxid = subxids[j];
-        i += j - i + 1;
+        i = j + 1;
     }
 
     /* update the cached value in shared memory */
-- 
2.27.0


pgsql-hackers by date:

Previous
From: Kyotaro Horiguchi
Date:
Subject: Re: Add checkpoint and redo LSN to LogCheckpointEnd log message
Next
From: Andres Freund
Date:
Subject: Re: Add last commit LSN to pg_last_committed_xact()