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

x4mmm@yandex-team.ru

From: Andrey Borodin <x4mmm@yandex-team.ru>
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
Date: 2020-10-24T16:40:35Z
Lists: pgsql-bugs

Attachments


> 22 окт. 2020 г., в 19:01, Tom Lane <tgl@sss.pgh.pa.us> написал(а):
> 
> Andrey Borodin <x4mmm@yandex-team.ru> writes:
>>> 13 окт. 2020 г., в 03:16, Tom Lane <tgl@sss.pgh.pa.us> написал(а):
>>> 2. The test exposed the gistRelocateBuildBuffersOnSplit bug only about
>>> one time in ten for me.  I suppose this is due to the random split
>>> choices gistchoose makes for equally-good tuples, so it's not terribly
>>> surprising; but it is problematic for a test that we're hoping to use
>>> to provide reliable code coverage.
>>> 
>>> I'm not really sure what we could do about #2.  Perhaps, instead of
>>> relying on random(), we could make gistchoose() use our own PRNG and
>>> then invent a debugging function that forces the seed to a known value.
>>> (GEQO already does something similar, although I'd not go as far as
>>> exposing the seed as a GUC.  Resetting it via some quick-hack C
>>> function in regress.c would be enough IMO.)  Or perhaps gist.sql could
>>> be adjusted so that the test data is less full of equally-good tuples.
> 
>> I think we should use not entropy-based tie breaker in GiST. We can extract some randomness from tuples using hash.
>> I'd be much happier if GiST behaviour was deterministic.
> 
> If we started using our own PRNG, we could very easily make the "random"
> sequence be the same in every GiST build --- again, see GEQO for prior
> art.  I'm a little suspicious of trying to pull some entropy out of the
> tuples themselves: that seems like it'd be tremendously prone to cross-
> platform and cross-version behavioral differences.

PFA copy of GEQO approach to GiST. I haven't found proper place to document this: current random tie breaker seems undocumented.
Will describing GUC here [0] be enough?

Thanks!

Best regards, Andrey Borodin.

[0] https://www.postgresql.org/docs/13/runtime-config-developer.html

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.