Re: pgstattuple: Use streaming read API in pgstatindex functions

Hi Xuneng Zhou

> - /* Unlock and release buffer */
> - LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
> - ReleaseBuffer(buffer);
> + UnlockReleaseBuffer(buffer);
> }
Thanks for your patch! Just to nitpick a bit — I think this comment is
worth keeping, even though the function name already conveys its meaning.

On Mon, Oct 13, 2025 at 4:42 PM Xuneng Zhou <xunengzhou(at)gmail(dot)com> wrote:

> Hi Bilal,
>
> Thanks for looking into this.
>
> On Mon, Oct 13, 2025 at 3:00 PM Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com>
> wrote:
> >
> > Hi,
> >
> > Thank you for working on this!
> >
> > On Mon, 13 Oct 2025 at 06:20, Xuneng Zhou <xunengzhou(at)gmail(dot)com> wrote:
> > >
> > > Fix indentation issue in v1.
> >
> > I did not look at the benchmarks, so here are my code comments.
> >
> > - I would avoid creating a new scope for the streaming read. While it
> > makes the streaming code easier to interpret, it introduces a large
> > diff due to indentation changes.
> >
> > - I suggest adding the following comment about streaming batching, as
> > it is used in other places:
> >
> > /*
> > * It is safe to use batchmode as block_range_read_stream_cb
> takes no
> > * locks.
> > */
> >
> > - For the '/* Scan all blocks except the metapage using streaming
> > reads */' comments, it might be helpful to clarify that the 0th page
> > is the metapage. Something like: '/* Scan all blocks except the
> > metapage (0th page) using streaming reads */'.
> >
> > Other than these comments, the code looks good to me.
> >
>
> Here is patch v3. The comments have been added, and the extra scope
> ({}) has been removed as suggested.
>
> Best,
> Xuneng
>