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 <[email protected]> wrote: > Hi Bilal, > > Thanks for looking into this. > > On Mon, Oct 13, 2025 at 3:00 PM Nazir Bilal Yavuz <[email protected]> > wrote: > > > > Hi, > > > > Thank you for working on this! > > > > On Mon, 13 Oct 2025 at 06:20, Xuneng Zhou <[email protected]> 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 >
