Re: regression coverage gaps for gist and hash indexes

Hi,

On 2023-03-31 10:45:51 +0300, Andrey Borodin wrote:
> On Fri, Mar 31, 2023 at 8:07 AM Andres Freund  wrote:
> >
> > I was working on committing patch 0001 from [1] and was a bit confused about
> > some of the changes to the WAL format for gist and hash index vacuum. It
> > looked to me that the changed code just flat out would not work.
> >
> > Turns out the problem is that we don't reach deletion for hash and gist
> > vacuum:
> >
> > gist:
> >
> > > Oh, I see. We apparently don't reach the gist deletion code in the tests:
> > > https://coverage.postgresql.org/src/backend/access/gist/gistxlog.c.gcov.html#674
> > > https://coverage.postgresql.org/src/backend/access/gist/gistxlog.c.gcov.html#174
> > >
> > > And indeed, if I add an abort() into , it's not reached.
> > >
> > > And it's not because tests use a temp table, the caller is also unreachable:
> > > https://coverage.postgresql.org/src/backend/access/gist/gist.c.gcov.html#1643
> >
> 
> GiST logs deletions in gistXLogUpdate(), which is covered.
> gistXLogDelete() is only used for cleaning during page splits.

I am not sure what your point here is - deleting entries to prevent a page
split is deleting entries. What am I missing?


> propose refactoring GiST WAL to remove gistXLogDelete() and using
> gistXLogUpdate() instead.

I think we still would need to have coverage for gistprunepage(), even if
so. So that seems a separate issue.

I think what gist.sql is missing is a combination of delete, index scan (to
mark entries dead), new insertions (to trigger pruning to prevent page
splits).

Greetings,

Andres Freund