Horton

Re: BUG #16329: Valgrind detects an invalid read when building a gist index with buffering

Alexander Law <exclusion@gmail.com>

View thread View on postgr.es
From: Alexander Lakhin <exclusion@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>, Pavel Borisov <pashkin.elfe@gmail.com>
Cc: pgsql-bugs@lists.postgresql.org, kyzevan23@mail.ru
Date: 2023-03-30T05:00:00Z
Lists: pgsql-bugs 
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

Commits

  1. Fix dereference of dangling pointer in GiST index buffering build.

  2. Fix GiST buffering build to work when there are included columns.

  3. Re-allow testing of GiST buffered builds.

  4. Add support for building GiST index by sorting.