From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alexander Korotkov <akorotkov@postgresql.org>
cc: pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Fix page modification outside of critical section in GIN
In-reply-to: <E1iXAuR-0001k9-Cn@gemulon.postgresql.org>
References: <E1iXAuR-0001k9-Cn@gemulon.postgresql.org>
Comments: In-reply-to Alexander Korotkov <akorotkov@postgresql.org>
	message dated "Tue, 19 Nov 2019 21:19:39 +0000"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <21619.1581098806.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Fri, 07 Feb 2020 13:06:46 -0500
Message-ID: <21620.1581098806@sss.pgh.pa.us>
Precedence: bulk

Alexander Korotkov <akorotkov@postgresql.org> writes:
> Fix page modification outside of critical section in GIN
> By oversight 52ac6cd2d0 makes ginDeletePage() sets pd_prune_xid of page =
to be
> deleted before entering the critical section.  It appears that only vers=
ions 11
> and later were affected by this oversight.

I happened to notice, while reviewing stuff for the release notes,
that this patch does not do what the commit message says.  The
previous code modified the pd_prune_xid of the "dBuffer" page,
but now it's modifying the pd_prune_xid of the "lBuffer" page.
Is that actually correct?  If it isn't, maybe you need to move
the GinPageSetDeleteXid call down a bit further, next to the
GinPageSetDeleted call (by which time the "page" variable has
been restored to the right thing).

Another idea which would be safer if a bit more invasive is to stop
using the single "page" variable for two purposes.

			regards, tom lane