MIME-Version: 1.0
References: <CALdSSPgZWX_D8+Fx4YQqRN5eW5iSx_rJdqQhCfdWTvqKXVfJ4w@mail.gmail.com>
 <CALdSSPgPs=gKWkKN0=4yBd49FBY=ijxBz4YGV64vfVMK7YQhpw@mail.gmail.com>
 <CALdSSPiaQptrccvRoHhP3=YqN9-K8duXfLHHm3G=+7FVL8A-Xw@mail.gmail.com>
 <CALdSSPh-QtT8NrhtKsNWwFVF3D4KX6Wy3qs-FPc16c6VAECEyg@mail.gmail.com>
 <CALdSSPgfFWUBc8RoOZe5PmoUGa4LJUDeMTcAT8HwXGCMGJ9uBA@mail.gmail.com>
 <449DFC8A-CF54-493E-9366-AF9C73C782AA@yandex-team.ru> <CALdSSPiGnqgvpfQDCydBf-=0hzyUs6Y0o0xNvd53BbcMWTVO=w@mail.gmail.com>
 <14B4E29D-7B08-4972-BE9E-491A8A6A77CF@yandex-team.ru>
In-Reply-To: <14B4E29D-7B08-4972-BE9E-491A8A6A77CF@yandex-team.ru>
From: Soumya S Murali <soumyamurali.work@gmail.com>
Date: Wed, 11 Feb 2026 17:48:39 +0530
Message-ID: <CAMtXxw_=fD6hAoLPagLT8fEPa6qe=oD-6ecXq2h1UmtBLv_GFA@mail.gmail.com>
Subject: Re: Fix gistkillitems & add regression test to microvacuum
To: Andrey Borodin <x4mmm@yandex-team.ru>
Cc: Kirill Reshke <reshkekirill@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAMtXxw_%3DfD6hAoLPagLT8fEPa6qe%3DoD-6ecXq2h1UmtBLv_GFA%40mail.gmail.com>
Precedence: bulk

Hi all,

Thank you for the updated patch.

On Wed, Feb 11, 2026 at 2:52=E2=80=AFPM Andrey Borodin <x4mmm@yandex-team.r=
u> wrote:
>
>
>
> > On 23 Jan 2026, at 16:03, Kirill Reshke <reshkekirill@gmail.com> wrote:
> >
> > On Tue, 20 Jan 2026 at 15:30, Andrey Borodin <x4mmm@yandex-team.ru> wro=
te:
> >>
> >>
> >>
> >>> On 15 Jan 2026, at 22:59, Kirill Reshke <reshkekirill@gmail.com> wrot=
e:
> >>>
> >>> 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 m=
aster 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 refact=
oring.
>
> >
> >> +# Test gist, but with fewer rows - that killitems used to be buggy.
> >>
> >> Probably, in this comment we can explicitly say that killitems was bug=
gy, 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