Hi,
On Sat, May 31, 2025 at 7:41 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>
> Here are a few review comments on the patch:
>
> + for (j = 0; j < nforks; j++)
> {
> - InvalidateLocalBuffer(bufHdr, true);
> + if ((buf_state & BM_TAG_VALID) &&
> + BufTagGetForkNum(&bufHdr->tag) == forkNum[j] &&
> + bufHdr->tag.blockNum >= firstDelBlock[j])
> + {
> + InvalidateLocalBuffer(bufHdr, true);
> + }
>
> It looks like the "buf_state & BM_TAG_VALID" check can be moved
> outside the loop, along with the BufTagMatchesRelFileLocator() check.
> That would avoid unnecessary looping.
>
> Also, should we add a "break" right after calling InvalidateLocalBuffer()?
> Since the buffer has already been invalidated, continuing the loop
> may not be necessary.
Thanks for the review! I'll fix both remarks. Please see the v2 patch.
--
Best regards,
Daniil Davydov