Subject: Re: Fix gistkillitems & add regression test to microvacuum

From: Andrey Borodin <[email-masked]>
Date: Tue, 20 Jan 2026 15:30:30 +0500

> On 15 Jan 2026, at 22:59, Kirill Reshke <[email-masked]> wrote:
> 
> PFA v2 which leaves the test in-place.
> 
> Also commit message improvements.

Yeah, killtuples for GiST root page is broken. Your patch is fixing it.
I don't think we should backpatch this, the bug is harmless, but for master the patch LGTM.
It would be good to assign so->curBlkno and so->curBlkno together. But gistScanPage() is the only place with access to the block number.

+# Test gist, but with fewer rows - that killitems used to be buggy.

Probably, in this comment we can explicitly say that killitems was buggy, but now is fixed.

Thanks!

Best regards, Andrey Borodin.

---

From: Kirill Reshke <[email-masked]>
Date: Fri, 23 Jan 2026 16:03:50 +0500

> I don't think we should backpatch this, the bug is harmless, but for master the patch LGTM.

Thank you

> It would be good to assign so->curBlkno and so->curBlkno together. But gistScanPage() is the only place with access to the block number.

Sorry, didnt get this take.

> Probably, in this comment we can explicitly say that killitems was buggy, but now is fixed.

Hmm, what would be a good wording here?

-- 
Best regards,
Kirill Reshke

---

From: Andrey Borodin <[email-masked]>
Date: Tue, 27 Jan 2026 12:26:01 +0500

> On 23 Jan 2026, at 16:03, Kirill Reshke <[email-masked]> wrote:
> 

Sorry, I meant so->curBlkno and so->numKilled are semantically correlated. But it's difficult to assign them together and this does not worth refactoring.

> Hmm, what would be a good wording here?

I've opened an English dictionary and it says "used to" can be used with a meaning "bug was there but it's not anymore".

So the patch is RfC IMO.

---

From: Soumya S Murali <[email-masked]>
Date: Wed, 11 Feb 2026 17:48:39 +0530

Hi all,

Thank you for the updated patch.

> I've opened an English dictionary and it says "used to" can be used with a meaning "bug was there but it's not anymore".

I reproduced the issue locally by filling a GiST root page, deleting
all tuples, and triggering a microvacuum by an index-only scan.
With the patch, The root page is now handled consistently with other
leaf pages. so->curBlkno and so->curPageLSN are properly set during
scan, and gistkillitems() operates safely and correctly on the root
page.
Based on runtime validation and code inspection, the fix LGTM.

Thanks,
Soumya

---

From: Kirill Reshke <[email-masked]>
Date: Wed, 18 Mar 2026 10:42:08 +0500

> Based on runtime validation and code inspection, the fix LGTM.

Rebased due to  f5eb854ab6d[0]

I have added a [0]committer to CC.

Andres, can you please take a look at this thread, if by chance you
have spare time for it? You have changed kill tuples-related logic
frequently, so you might be interested...

[0] https://git.postgresql.org/cgit/postgresql.git/commit/?id=f5eb854ab6d6281ec2d3143657944bdda6676341

-- 
Best regards,
Kirill Reshke

---

From: Soumya S Murali <[email-masked]>
Date: Fri, 20 Mar 2026 11:47:04 +0530

Hi all,

> [0] https://git.postgresql.org/cgit/postgresql.git/commit/?id=f5eb854ab6d6281ec2d3143657944bdda6676341

I performed a follow-up test on top of the v3 patch with a strictly
controlled setup to ensure a single-page GiST index (confirming that
block 1 is out of range). To force the cleanup path, I executed an
index-only scan after deleting all tuples and used a narrow WAL window
to isolate the relevant records. The WAL output shows GiST PAGE_UPDATE
on block 0, confirming that the root page is modified after the scan.
However even in this setup, I do not observe any XLOG_GIST_DELETE
records. This seems while the patch correctly initializes curBlkno and
enables proper handling of the root page, the DELETE WAL emission
depends on whether killitems actually marks tuples as DEAD under the
given visible conditions.
Overall, the fix looks correct from a code perspective, but in this
scenario the cleanup does not result in a DELETE WAL record.

Regards,
Soumya