From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
cc: =?utf-8?B?55uP5LiA?= <w@hidva.com>,
        pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: use `proc->pgxactoff` as the value of `index` in
 `ProcArrayRemove()`
In-reply-to: <20210506190841.sqxci46qamoyb55u@alap3.anarazel.de>
References: <tencent_5624AA3B116B3D1C31CA9744@qq.com>
 <20210506190841.sqxci46qamoyb55u@alap3.anarazel.de>
Comments: In-reply-to Andres Freund <andres@anarazel.de>
	message dated "Thu, 06 May 2021 12:08:41 -0700"
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-ID: <4175577.1620329249.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Thu, 06 May 2021 15:27:29 -0400
Message-ID: <4175578.1620329249@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/4175578.1620329249%40sss.pgh.pa.us>
Precedence: bulk

Andres Freund <andres@anarazel.de> writes:
> On 2021-05-07 00:30:13 +0800, =E7=9B=8F=E4=B8=80 wrote:
>> Since we have introduced `pgxactoff` in [941697c3c1ae5d6ee153065adb96e1=
e63ee11224](https://github.com/postgres/postgres/commit/941697c3c1ae5d6ee1=
53065adb96e1e63ee11224), and `pgxactoff` is always the index of `proc->pgp=
rocno` in `procArray->pgprocnos`. So it seems that we could directly use `=
proc->pgxactoff` as the value of `index` in `ProcArrayRemove()`? My though=
t is to replace

> Sounds like a plan! Do you want to write a patch?

> If you do, I think it might be worthwhile to add an only-with-assertions
> loop checking that there's no other entry with the same pgprocno in the
> dense arrays.

Hmm, I can definitely see keeping a check that the selected entry
has the right PID and/or pgprocno, but making it search for duplicates
seems a bit over the top.  The existing code isn't guarding against
that, and I don't really see a reason why there's a meaningful risk
of it.

> Given that the code is new in 14, I wonder if we should cram this
> simplification in before beta?

+1, seems like a pretty clear missed opportunity in 941697c3c.

			regards, tom lane