On Mon, Oct 14, 2024 at 6:30 PM Ilia Evdokimov <ilya.evdokimov@tantorlabs.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@mail.gmail.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.
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.
