Re: Use function smgrclose() to replace the loop

Junwang Zhao <zhjwpku(at)gmail(dot)com> 于2024年10月15日周二 18:56写道：

> On Mon, Oct 14, 2024 at 6:30 PM Ilia Evdokimov
> <ilya(dot)evdokimov(at)tantorlabs(dot)com> wrote:
> >
> >
> > On 14.08.2024 09:32, Steven Niu wrote:
> > > Hi, Kirill, Junwang,
> > >
> > > I made this patch to address the refactor issue in our previous email
> > > discussion.
> > >
> https://www.postgresql.org/message-id/flat/CABBtG=cDTCBDCBK7McSy6bJR3s5xUTOg0vSFfuW8oLdUYyCscA(at)mail(dot)gmail(dot)com
> > >
> > >
> > > That is, the for loop in function smgrdestroy() and smgrdounlinkall
> > > can be replaced with smgrclose().
> > >
> > > for (forknum = 0; forknum <= MAX_FORKNUM; forknum++)
> > > smgrsw[reln->smgr_which].smgr_close(reln, forknum);
> > > -->
> > > smgrclose(rels[i]);
> > >
> > > Please let me know if you have any questions.
> > >
> > > Best Regards,
> > > Steven from Highgo.com
> >
> > Hello,
> >
> > Are you sure we can refactor loop by 'smgrclose()'? I see it is not
> > quite the same.
>
> smgrclose does more by setting smgr_cached_nblocks[] and smgr_targblock
> to InvalidBlockNumber, I see no harm with the replacement, not 100% sure
> though.
>
> >
> > Regards,
> > Ilia Evdokimov,
> > Tantor Labs LLC.
> >
>
>
> --
> Regards
> Junwang Zhao
>

When I look into smgrrelease(), which also resets smgr_cached_nblocks and
smgr_targblock, I would say this is of good style.
So a bare loop of calling smgr_close() without other cleanup actions is
kind of weird to me. Unless there is some reason for the current code, I'd
like to replace it.

Regards,
Steven