Use function smgrclose() to replace the loop
Hi, Kirill, Junwang,
I made this patch to address the refactor issue in our previous email
discussion.
/messages/by-id/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
Attachments:
0001-Use-function-smgrclose-to-replace-the-loop.patchapplication/octet-stream; name=0001-Use-function-smgrclose-to-replace-the-loop.patchDownload
From 3929d3771a21b7cfa41d78868409cfa7ea702fe8 Mon Sep 17 00:00:00 2001
From: Steven Niu <niushiji@gmail.com>
Date: Tue, 13 Aug 2024 19:12:37 -0700
Subject: Use function smgrclose() to replace the loop
Use function smgrclose() to replace the loop to simplify code.
Author: Steven Niu <niushiji@gmail.com>
---
src/backend/storage/smgr/smgr.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c
index 7b9fa103ef..0d6a0f9543 100644
--- a/src/backend/storage/smgr/smgr.c
+++ b/src/backend/storage/smgr/smgr.c
@@ -280,8 +280,7 @@ smgrdestroy(SMgrRelation reln)
Assert(reln->pincount == 0);
- for (forknum = 0; forknum <= MAX_FORKNUM; forknum++)
- smgrsw[reln->smgr_which].smgr_close(reln, forknum);
+ smgrclose(reln);
dlist_delete(&reln->node);
@@ -487,8 +486,7 @@ smgrdounlinkall(SMgrRelation *rels, int nrels, bool isRedo)
rlocators[i] = rlocator;
/* Close the forks at smgr level */
- for (forknum = 0; forknum <= MAX_FORKNUM; forknum++)
- smgrsw[which].smgr_close(rels[i], forknum);
+ smgrclose(rels[i]);
}
/*
--
2.43.0
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.
/messages/by-id/CABBtG=cDTCBDCBK7McSy6bJR3s5xUTOg0vSFfuW8oLdUYyCscA@mail.gmail.comThat 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.
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.
/messages/by-id/CABBtG=cDTCBDCBK7McSy6bJR3s5xUTOg0vSFfuW8oLdUYyCscA@mail.gmail.comThat 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.comHello,
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
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./messages/by-id/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.comHello,
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