| From: | Soumya S Murali <soumyamurali(dot)work(at)gmail(dot)com> |
|---|---|
| To: | Andrey Borodin <x4mmm(at)yandex-team(dot)ru> |
| Cc: | Kirill Reshke <reshkekirill(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Fix gistkillitems & add regression test to microvacuum |
| Date: | 2026-02-11 12:18:39 |
| Message-ID: | CAMtXxw_=fD6hAoLPagLT8fEPa6qe=oD-6ecXq2h1UmtBLv_GFA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi all,
Thank you for the updated patch.
On Wed, Feb 11, 2026 at 2:52 PM Andrey Borodin <x4mmm(at)yandex-team(dot)ru> wrote:
>
>
>
> > On 23 Jan 2026, at 16:03, Kirill Reshke <reshkekirill(at)gmail(dot)com> wrote:
> >
> > On Tue, 20 Jan 2026 at 15:30, Andrey Borodin <x4mmm(at)yandex-team(dot)ru> wrote:
> >>
> >>
> >>
> >>> On 15 Jan 2026, at 22:59, Kirill Reshke <reshkekirill(at)gmail(dot)com> 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.
> >
> > 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.
>
> 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.
>
> >
> >> +# 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.
> >>
> >
> > 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.
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 | Date | Subject | |
|---|---|---|---|
| Next Message | Hayato Kuroda (Fujitsu) | 2026-02-11 12:23:42 | RE: parallel data loading for pgbench -i |
| Previous Message | Mircea Cadariu | 2026-02-11 12:15:56 | Re: Propagate XLogFindNextRecord error to callers |