Horton

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

Noah Misch <noah@leadboat.com>

View thread View on postgr.es
From: Noah Misch <noah@leadboat.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Pavel Borisov <pashkin.elfe@gmail.com>, Alexander Lakhin <exclusion@gmail.com>, pgsql-bugs@lists.postgresql.org, kyzevan23@mail.ru
Date: 2023-03-30T04:42:45Z
Lists: pgsql-bugs 
On Wed, Mar 29, 2023 at 06:21:38PM -0400, Tom Lane wrote:
> *** a/src/test/regress/sql/gist.sql
> --- b/src/test/regress/sql/gist.sql
> *************** select p from gist_tbl order by circle(p
> *** 170,175 ****
> --- 170,180 ----
>   select p from gist_tbl order by circle(p,1) <-> point(0,0) limit 1;
>   
>   -- Force an index build using buffering.
> + insert into gist_tbl
> + select box(point(0.05*i, 0.05*i), point(0.05*i, 0.05*i)),
> +        point(0.05*i, 0.05*i),
> +        circle(point(0.05*i, 0.05*i), 1.0)
> + from generate_series(10001,30000) as i;
>   create index gist_tbl_box_index_forcing_buffering on gist_tbl using gist (p)
>     with (buffering=on, fillfactor=50);
>   
> 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.

I say +200ms is fine.  This thread is a reminder of the limits of raw code
coverage as a measure of tests.  (I expect this will add tens of minutes to
the soft-float buildfarm animal runs, but they deserve it.)

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.