Re: GiST nitpicks I want to discuss (and maybe eventually fix)

Hi Kirill!

> On 6 Oct 2025, at 22:57, Kirill Reshke <reshkekirill(at)gmail(dot)com> wrote:
>
> 0001:
> Remove GistTuplesDeleted support from GiST
>
> This bit flag value is unused since
> 68446b2c87a2aee5d8c2eb2aade7bb6d4195b7e1.
> F_TUPLES_DELETED flag is not removed, since it is still used for
> pageinspect. This is because we will still have this flag in mask
> after a major upgrade, so we should keep page inspect code that
> displays it. Patch merely suggests not to set it anymore.

Makes sense. I hope one day we will add a catalog field to track index creation version. This would pave the way to get rid of invalid GiST tuples and return this flag too. We can use it for something better.
FTR, in commit message you could mention shorter version of hash 68446b2.

>
> (there was 0002, but I self-rejected it, because it is too picky)

Out of curiosity, what was that?

>
> 0003:
>
> Remove block field from gistxlogPageReuse walrecord.
> GIST_REUSE_PAGE is spectail WAL record. It does not reference any
> page, does not change any page in redo, its sole purpose is to kill
> standby-queries which might still observe GiST index page we want to
> reuse. It has `block` field which is simply not used for any purpose.
> I propose to remove this field, reducing WAL footprint by an enormous
> 4 byte-per-record.

Yes, this is Hot-standby-only record. In offline conversation you mentioned that "RelFileLocator locator;" is not needed, only "Oid dbOid;". Maybe let's save a little more bytes?

>
> 0004:
> Reduce GIST_MAX_SPLIT_PAGES to some sane limit.
>
> Value of GIST_MAX_SPLIT_PAGES is too big. There is no chance that GiST
> page split will successfully produce 75 new pages, because there is an
> upper limit of how many backup blocks a single WAL record can
> reference (XLR_MAX_BLOCK_ID = 32). Patch reduces to
> GIST_MAX_SPLIT_PAGES and adds static assertion for value.
>
> Some history:
> commit introducing GIST_MAX_SPLIT_PAGES [0]
> commit introducing XLR_MAX_BLOCK_ID [1]
>
> [0] was committed before [1]
>
>
>
> I was able to create a case where split produces 6 pages. I was not
> able to do more. So, I dont know if there is any real-life case where
> this problem reproduces.

IMHO, static assert is too fancy. Why not just #define GIST_MAX_SPLIT_PAGES (XLR_MAX_BLOCK_ID - 1) ?

Overall changes seem valuable, reducing GiST code complexity. Thank you!

Best regards, Andrey Borodin.