Re: regression coverage gaps for gist and hash indexes

Hi!

On Fri, Mar 31, 2023 at 8:07 AM Andres Freund <andres(at)anarazel(dot)de> 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'd
propose refactoring GiST WAL to remove gistXLogDelete() and using
gistXLogUpdate() instead.
However I see that gistXLogPageDelete() is not exercised, and is worth
fixing IMO. Simply adding 10x more data in gist.sql helps, but I think
we can do something more clever...

Best regards, Andrey Borodin.