From: Alexander Lakhin <exclusion@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
To: Pavel Borisov <pashkin.elfe@gmail.com>
Cc: pgsql-bugs@lists.postgresql.org
Cc: kyzevan23@mail.ru
Subject: Re: BUG #16329: Valgrind detects an invalid read when building a gist index with buffering
Date: Thu, 30 Mar 2023 08:00:00 +0300
Message-ID: <cad7055f-0d76-cc31-71d5-f8b600ebb116@gmail.com> (raw)
In-Reply-To: <4095379.1680128498@sss.pgh.pa.us>
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>
<3990135.1680104273@sss.pgh.pa.us>
<4095379.1680128498@sss.pgh.pa.us>
30.03.2023 01:21, Tom Lane wrote:
>
> The question is whether it's worth the extra test cycles forevermore.
> On my machine, the time for the gist test script goes from ~400ms to
> ~600ms. That's still less than some of the concurrent scripts (brin,
> privileges, and join_hash all take more than that for me), so maybe
> it's fine. But it seems like a lot for a change that doesn't move the
> raw code coverage results by even one line.
>
> Thoughts?
I would say that providing the behavior coverage is the next thing to do after
the code coverage reached 100%. I can easily imagine bugs hiding in a fully
covered code (#17858 is the recent example).
As to that concrete place, I would incline to improve the test while it stays
in line with other tests, but I lack a whole picture of the gist testing.
It doesn't seem to me that the regress/sql/gist.sql is too comprehensive.
I've just looked at the scan-build's results and found 9 warnings related to
gist. A quick glance at the code confirms that there are indeed
interesting cases, which are not exercised for now (e.g., call
gistRelocateBuildBuffersOnSplit() with
IndexRelationGetNumberOfKeyAttributes(r) > 1).
So if I could prove that 2-3 of them are really bugs during the next week,
maybe then assess that test change alongside with others (if any).
(Maybe we'll want to build that index over different columns, for example.)
Best regards,
Alexander
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: exclusion@gmail.com, tgl@sss.pgh.pa.us, pashkin.elfe@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: <cad7055f-0d76-cc31-71d5-f8b600ebb116@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox