Hi! On Fri, Mar 31, 2023 at 8:07 AM Andres Freund <and...@anarazel.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.