From: Tom Lane <tgl@sss.pgh.pa.us>
To: Pavel Borisov <pashkin.elfe@gmail.com>
cc: Alexander Lakhin <exclusion@gmail.com>, pgsql-bugs@lists.postgresql.org,
        kyzevan23@mail.ru
Subject: Re: BUG #16329: Valgrind detects an invalid read when building a gist
 index with buffering
In-reply-to: 
 <CALT9ZEEJ1pqAnkcO4H7AWOBhLN0KJDa9A=pDn9NC7pbqkwEAeQ@mail.gmail.com>
References: <16329-7a6aa9b6fa1118a1@postgresql.org>
 <ca1f896b-e5d1-53a7-5bc3-9451ab9aca22@gmail.com>
 <e91e45fd-a614-bb87-e9fc-c556c2b95fe5@gmail.com>
 <3dda6945-c147-5afc-7720-38ec6848dafa@gmail.com>
 <10261.1588705157@sss.pgh.pa.us>
 <CALT9ZEEJ1pqAnkcO4H7AWOBhLN0KJDa9A=pDn9NC7pbqkwEAeQ@mail.gmail.com>
Comments: In-reply-to Pavel Borisov <pashkin.elfe@gmail.com>
	message dated "Mon, 02 Nov 2020 20:55:14 +0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3990134.1680104273.1@sss.pgh.pa.us>
Date: Wed, 29 Mar 2023 11:37:53 -0400
Message-ID: <3990135.1680104273@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/3990135.1680104273%40sss.pgh.pa.us>
Precedence: bulk

Pavel Borisov <pashkin.elfe@gmail.com> writes:
> So I think that the patch by Alexander will do the thing. I also added some
> comments to the code and removed extra context reset in gistBuildCallback
> (which is already done level down) to make things clear.

My apologies for having let this slip through the cracks.  I think
I'd wanted to understand why the committed version of the GiST
index tests doesn't expose the problem, and I never got time to
study that.  I still haven't tracked it down, but the proposed patch
seems clearly safe so I've gone ahead and pushed it.

I did use Alexander's original code change that just moved the
totals-incrementing.  It looks to me like gistProcessEmptyingQueue
is not guaranteed to reach the context reset, and I'd rather have
an extra reset than miss one.  (No-op context resets are cheap.)

			regards, tom lane