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

Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com> writes:
>> I investigated this issue under Valgrind and found that in v13 it is no
>> longer detected and the patch in the discussion above is no longer needed.

> I want to correct myself that issue is no longer detected in master, but
> not in 13stable. Everything else is without changes.

I went to try to evaluate this, and found that with or without your
proposed testing addition, the code coverage for gistbuildbuffers.c
remains at 0.0%. The previous testing patch is the same. So that's
why the issue is "no longer detectable": the code is not reached in
HEAD.

After some investigation, I found that commit 16fa9b2b3 broke this:
if the opclass(es) have GIST_SORTSUPPORT_PROC, then the buffering
code will not be used, no matter what the index's options say.

This seems like a spectacularly bad idea from a testing standpoint,
even if it's the right thing for most users. Basically, it is now
impossible to test buffering builds at all, unless you find a gist
opclass that lacks GIST_SORTSUPPORT_PROC. Although there are a
few candidates to pick from, someone could at any time add such
a support proc and silently break your testing plan, just as
16fa9b2b3 did by adding such a proc to point_ops.

So I think 16fa9b2b3 has to be reconsidered: if we have a
buffering=on index parameter, we must go with that independently
of the availability of sort support procs. Unless I hear very
loud squawks very quickly, I'll go make that happen.

(I observe that 16fa9b2b3 also failed to update the relevant
documentation in 65.4.1...)

regards, tom lane