Postgres Pro
Downloads
Home   >   mailing lists

Thread: Use function smgrclose() to replace the loop

Use function smgrclose() to replace the loop

From
Steven Niu
Date:
Hi, Kirill, Junwang,

I made this patch to address the refactor issue in our previous email discussion.

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
Attachment

Re: Use function smgrclose() to replace the loop

From
Ilia Evdokimov
Date:
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.

Regards,
Ilia Evdokimov,
Tantor Labs LLC.

Re: Use function smgrclose() to replace the loop

From
Junwang Zhao
Date:
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.

>
> Regards,
> Ilia Evdokimov,
> Tantor Labs LLC.
>


--
Regards
Junwang Zhao

Re: Use function smgrclose() to replace the loop

From
Steven Niu
Date:


Junwang Zhao <zhjwpku@gmail.com> 于2024年10月15日周二 18:56写道：
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.

>
> 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