public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v2] Use SMgrRelation instead of SMgrRelationData * in pgaio_io_set_target_smgr
8+ messages / 2 participants
[nested] [flat]

* [PATCH v2] Use SMgrRelation instead of SMgrRelationData * in pgaio_io_set_target_smgr
@ 2026-03-13 06:55  Japin Li <[email protected]>
  0 siblings, 0 replies; 8+ messages in thread

From: Japin Li @ 2026-03-13 06:55 UTC (permalink / raw)

To keep API consistency with the rest of the smgr subsystem, change the
parameter type from SMgrRelationData * to SMgrRelation in both the
declaration and definition of pgaio_io_set_target_smgr().

This commit also adds the const qualifier to the SMgrRelation since the
function only reads the relation and never modifies it, marking the parameter
as const is more accurate and improves code clarity and safety.
---
 src/backend/storage/smgr/smgr.c | 2 +-
 src/include/storage/smgr.h      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c
index 5391640d861..abf6b219abf 100644
--- a/src/backend/storage/smgr/smgr.c
+++ b/src/backend/storage/smgr/smgr.c
@@ -1036,7 +1036,7 @@ ProcessBarrierSmgrRelease(void)
  */
 void
 pgaio_io_set_target_smgr(PgAioHandle *ioh,
-						 SMgrRelationData *smgr,
+						 const SMgrRelation smgr,
 						 ForkNumber forknum,
 						 BlockNumber blocknum,
 						 int nblocks,
diff --git a/src/include/storage/smgr.h b/src/include/storage/smgr.h
index 09bd42fcf4b..2f982deb6df 100644
--- a/src/include/storage/smgr.h
+++ b/src/include/storage/smgr.h
@@ -135,7 +135,7 @@ smgrwrite(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
 }
 
 extern void pgaio_io_set_target_smgr(PgAioHandle *ioh,
-									 SMgrRelationData *smgr,
+									 const SMgrRelation smgr,
 									 ForkNumber forknum,
 									 BlockNumber blocknum,
 									 int nblocks,
-- 
2.43.0


--=-=-=--





^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* [PATCH v1] Use SMgrRelation instead of SMgrRelationData * in pgaio_io_set_target_smgr
@ 2026-03-13 06:55  Japin Li <[email protected]>
  0 siblings, 0 replies; 8+ messages in thread

From: Japin Li @ 2026-03-13 06:55 UTC (permalink / raw)

To keep API consistency with the rest of the smgr subsystem, change the
parameter type from SMgrRelationData * to SMgrRelation in both the
declaration and definition of pgaio_io_set_target_smgr().
---
 src/backend/storage/smgr/smgr.c | 2 +-
 src/include/storage/smgr.h      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c
index 5391640d861..e5822fa8d92 100644
--- a/src/backend/storage/smgr/smgr.c
+++ b/src/backend/storage/smgr/smgr.c
@@ -1036,7 +1036,7 @@ ProcessBarrierSmgrRelease(void)
  */
 void
 pgaio_io_set_target_smgr(PgAioHandle *ioh,
-						 SMgrRelationData *smgr,
+						 SMgrRelation smgr,
 						 ForkNumber forknum,
 						 BlockNumber blocknum,
 						 int nblocks,
diff --git a/src/include/storage/smgr.h b/src/include/storage/smgr.h
index 09bd42fcf4b..e2fc7ba1bcb 100644
--- a/src/include/storage/smgr.h
+++ b/src/include/storage/smgr.h
@@ -135,7 +135,7 @@ smgrwrite(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
 }
 
 extern void pgaio_io_set_target_smgr(PgAioHandle *ioh,
-									 SMgrRelationData *smgr,
+									 SMgrRelation smgr,
 									 ForkNumber forknum,
 									 BlockNumber blocknum,
 									 int nblocks,
-- 
2.43.0


--=-=-=--





^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Use SMgrRelation instead of SMgrRelationData * in pgaio_io_set_target_smgr()
@ 2026-03-13 07:05  Japin Li <[email protected]>
  0 siblings, 1 reply; 8+ messages in thread

From: Japin Li @ 2026-03-13 07:05 UTC (permalink / raw)
  To: PostgreSQL Hackers <[email protected]>


Hi, hackers

While reading smgr.h, I noticed an inconsistent type usage in
pgaio_io_set_target_smgr(). Currently the function is declared as:

    extern void pgaio_io_set_target_smgr(PgAioHandle *ioh,
                                         SMgrRelationData *smgr,
                                         ForkNumber forknum,
                                         BlockNumber blocknum,
                                         int nblocks,

However, SMgrRelation is defined as "typedef SMgrRelationData * SMgrRelation;",
and all other functions in the smgr subsystem use SMgrRelation as the parameter
type.

To keep the code consistent with the rest of the smgr API, this patch changes
the parameter from SMgrRelationData * to SMgrRelation in both the definition
and declaration.

This is purely a style/consistency cleanup with no functional change.

Thoughts? Is this change acceptable?

-- 
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.



^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: Use SMgrRelation instead of SMgrRelationData * in pgaio_io_set_target_smgr()
@ 2026-03-13 07:29  Chao Li <[email protected]>
  parent: Japin Li <[email protected]>
  0 siblings, 1 reply; 8+ messages in thread

From: Chao Li @ 2026-03-13 07:29 UTC (permalink / raw)
  To: Japin Li <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>



> On Mar 13, 2026, at 15:05, Japin Li <[email protected]> wrote:
> 
> 
> Hi, hackers
> 
> While reading smgr.h, I noticed an inconsistent type usage in
> pgaio_io_set_target_smgr(). Currently the function is declared as:
> 
>    extern void pgaio_io_set_target_smgr(PgAioHandle *ioh,
>                                         SMgrRelationData *smgr,
>                                         ForkNumber forknum,
>                                         BlockNumber blocknum,
>                                         int nblocks,
> 
> However, SMgrRelation is defined as "typedef SMgrRelationData * SMgrRelation;",
> and all other functions in the smgr subsystem use SMgrRelation as the parameter
> type.
> 
> To keep the code consistent with the rest of the smgr API, this patch changes
> the parameter from SMgrRelationData * to SMgrRelation in both the definition
> and declaration.
> 
> This is purely a style/consistency cleanup with no functional change.
> 
> Thoughts? Is this change acceptable?
> 
> -- 
> Regards,
> Japin Li
> ChengDu WenWu Information Technology Co., Ltd.
> 
> <v1-0001-Use-SMgrRelation-instead-of-SMgrRelationData-in-p.patch>

pgaio_io_set_target_smgr doesn’t update smgr, so, instead of “SMgrRelation”,  I think it’s better to change the type to "const SMgrRelationData *”.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/









^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: Use SMgrRelation instead of SMgrRelationData * in pgaio_io_set_target_smgr()
@ 2026-03-14 03:07  Japin Li <[email protected]>
  parent: Chao Li <[email protected]>
  0 siblings, 1 reply; 8+ messages in thread

From: Japin Li @ 2026-03-14 03:07 UTC (permalink / raw)
  To: Chao Li <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>


Hi, Chao

On Fri, 13 Mar 2026 at 15:29, Chao Li <[email protected]> wrote:
>> On Mar 13, 2026, at 15:05, Japin Li <[email protected]> wrote:
>> 
>> 
>> Hi, hackers
>> 
>> While reading smgr.h, I noticed an inconsistent type usage in
>> pgaio_io_set_target_smgr(). Currently the function is declared as:
>> 
>>    extern void pgaio_io_set_target_smgr(PgAioHandle *ioh,
>>                                         SMgrRelationData *smgr,
>>                                         ForkNumber forknum,
>>                                         BlockNumber blocknum,
>>                                         int nblocks,
>> 
>> However, SMgrRelation is defined as "typedef SMgrRelationData * SMgrRelation;",
>> and all other functions in the smgr subsystem use SMgrRelation as the parameter
>> type.
>> 
>> To keep the code consistent with the rest of the smgr API, this patch changes
>> the parameter from SMgrRelationData * to SMgrRelation in both the definition
>> and declaration.
>> 
>> This is purely a style/consistency cleanup with no functional change.
>> 
>> Thoughts? Is this change acceptable?
>> 
>> -- 
>> Regards,
>> Japin Li
>> ChengDu WenWu Information Technology Co., Ltd.
>> 
>> <v1-0001-Use-SMgrRelation-instead-of-SMgrRelationData-in-p.patch>
>
> pgaio_io_set_target_smgr doesn’t update smgr, so, instead of “SMgrRelation”,  I think it’s better to change the type to "const SMgrRelationData *”.

Thanks for the review! Makes sense — I've updated the patch to v2.

-- 
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.



^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: Use SMgrRelation instead of SMgrRelationData * in pgaio_io_set_target_smgr()
@ 2026-03-16 00:50  Chao Li <[email protected]>
  parent: Japin Li <[email protected]>
  0 siblings, 1 reply; 8+ messages in thread

From: Chao Li @ 2026-03-16 00:50 UTC (permalink / raw)
  To: Japin Li <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>



> On Mar 14, 2026, at 11:07, Japin Li <[email protected]> wrote:
> 
> 
> Hi, Chao
> 
> On Fri, 13 Mar 2026 at 15:29, Chao Li <[email protected]> wrote:
>>> On Mar 13, 2026, at 15:05, Japin Li <[email protected]> wrote:
>>> 
>>> 
>>> Hi, hackers
>>> 
>>> While reading smgr.h, I noticed an inconsistent type usage in
>>> pgaio_io_set_target_smgr(). Currently the function is declared as:
>>> 
>>>   extern void pgaio_io_set_target_smgr(PgAioHandle *ioh,
>>>                                        SMgrRelationData *smgr,
>>>                                        ForkNumber forknum,
>>>                                        BlockNumber blocknum,
>>>                                        int nblocks,
>>> 
>>> However, SMgrRelation is defined as "typedef SMgrRelationData * SMgrRelation;",
>>> and all other functions in the smgr subsystem use SMgrRelation as the parameter
>>> type.
>>> 
>>> To keep the code consistent with the rest of the smgr API, this patch changes
>>> the parameter from SMgrRelationData * to SMgrRelation in both the definition
>>> and declaration.
>>> 
>>> This is purely a style/consistency cleanup with no functional change.
>>> 
>>> Thoughts? Is this change acceptable?
>>> 
>>> -- 
>>> Regards,
>>> Japin Li
>>> ChengDu WenWu Information Technology Co., Ltd.
>>> 
>>> <v1-0001-Use-SMgrRelation-instead-of-SMgrRelationData-in-p.patch>
>> 
>> pgaio_io_set_target_smgr doesn’t update smgr, so, instead of “SMgrRelation”,  I think it’s better to change the type to "const SMgrRelationData *”.
> 
> Thanks for the review! Makes sense — I've updated the patch to v2.
> 
> -- 
> Regards,
> Japin Li
> ChengDu WenWu Information Technology Co., Ltd.
> 
> <v2-0001-Use-SMgrRelation-instead-of-SMgrRelationData-in-p.patch>

“const SMgrRelation *” will not work as you expected, you have to do “const SMgrRelationData *”. We want to protect the data the pointer pointing to from changing but the pointer itself.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/









^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: Use SMgrRelation instead of SMgrRelationData * in pgaio_io_set_target_smgr()
@ 2026-03-16 02:31  Japin Li <[email protected]>
  parent: Chao Li <[email protected]>
  0 siblings, 1 reply; 8+ messages in thread

From: Japin Li @ 2026-03-16 02:31 UTC (permalink / raw)
  To: Chao Li <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>

On Mon, 16 Mar 2026 at 08:50, Chao Li <[email protected]> wrote:
>> On Mar 14, 2026, at 11:07, Japin Li <[email protected]> wrote:
>> 
>> 
>> Hi, Chao
>> 
>> On Fri, 13 Mar 2026 at 15:29, Chao Li <[email protected]> wrote:
>>>> On Mar 13, 2026, at 15:05, Japin Li <[email protected]> wrote:
>>>> 
>>>> 
>>>> Hi, hackers
>>>> 
>>>> While reading smgr.h, I noticed an inconsistent type usage in
>>>> pgaio_io_set_target_smgr(). Currently the function is declared as:
>>>> 
>>>>   extern void pgaio_io_set_target_smgr(PgAioHandle *ioh,
>>>>                                        SMgrRelationData *smgr,
>>>>                                        ForkNumber forknum,
>>>>                                        BlockNumber blocknum,
>>>>                                        int nblocks,
>>>> 
>>>> However, SMgrRelation is defined as "typedef SMgrRelationData * SMgrRelation;",
>>>> and all other functions in the smgr subsystem use SMgrRelation as the parameter
>>>> type.
>>>> 
>>>> To keep the code consistent with the rest of the smgr API, this patch changes
>>>> the parameter from SMgrRelationData * to SMgrRelation in both the definition
>>>> and declaration.
>>>> 
>>>> This is purely a style/consistency cleanup with no functional change.
>>>> 
>>>> Thoughts? Is this change acceptable?
>>>> 
>>>> -- 
>>>> Regards,
>>>> Japin Li
>>>> ChengDu WenWu Information Technology Co., Ltd.
>>>> 
>>>> <v1-0001-Use-SMgrRelation-instead-of-SMgrRelationData-in-p.patch>
>>> 
>>> pgaio_io_set_target_smgr doesn’t update smgr, so, instead of “SMgrRelation”,  I think it’s better to change the type to "const SMgrRelationData *”.
>> 
>> Thanks for the review! Makes sense — I've updated the patch to v2.
>> 
>> -- 
>> Regards,
>> Japin Li
>> ChengDu WenWu Information Technology Co., Ltd.
>> 
>> <v2-0001-Use-SMgrRelation-instead-of-SMgrRelationData-in-p.patch>
>
> “const SMgrRelation *” will not work as you expected, you have to do “const SMgrRelationData *”. We want to protect the data the pointer pointing to from changing but the pointer itself.

Thanks for pointing that out!  I hadn't noticed the difference before.
Updated as you suggested.

>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/

-- 
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.



^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: Use SMgrRelation instead of SMgrRelationData * in pgaio_io_set_target_smgr()
@ 2026-03-16 03:14  Chao Li <[email protected]>
  parent: Japin Li <[email protected]>
  0 siblings, 0 replies; 8+ messages in thread

From: Chao Li @ 2026-03-16 03:14 UTC (permalink / raw)
  To: Japin Li <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>



> On Mar 16, 2026, at 10:31, Japin Li <[email protected]> wrote:
> 
> On Mon, 16 Mar 2026 at 08:50, Chao Li <[email protected]> wrote:
>>> On Mar 14, 2026, at 11:07, Japin Li <[email protected]> wrote:
>>> 
>>> 
>>> Hi, Chao
>>> 
>>> On Fri, 13 Mar 2026 at 15:29, Chao Li <[email protected]> wrote:
>>>>> On Mar 13, 2026, at 15:05, Japin Li <[email protected]> wrote:
>>>>> 
>>>>> 
>>>>> Hi, hackers
>>>>> 
>>>>> While reading smgr.h, I noticed an inconsistent type usage in
>>>>> pgaio_io_set_target_smgr(). Currently the function is declared as:
>>>>> 
>>>>>  extern void pgaio_io_set_target_smgr(PgAioHandle *ioh,
>>>>>                                       SMgrRelationData *smgr,
>>>>>                                       ForkNumber forknum,
>>>>>                                       BlockNumber blocknum,
>>>>>                                       int nblocks,
>>>>> 
>>>>> However, SMgrRelation is defined as "typedef SMgrRelationData * SMgrRelation;",
>>>>> and all other functions in the smgr subsystem use SMgrRelation as the parameter
>>>>> type.
>>>>> 
>>>>> To keep the code consistent with the rest of the smgr API, this patch changes
>>>>> the parameter from SMgrRelationData * to SMgrRelation in both the definition
>>>>> and declaration.
>>>>> 
>>>>> This is purely a style/consistency cleanup with no functional change.
>>>>> 
>>>>> Thoughts? Is this change acceptable?
>>>>> 
>>>>> -- 
>>>>> Regards,
>>>>> Japin Li
>>>>> ChengDu WenWu Information Technology Co., Ltd.
>>>>> 
>>>>> <v1-0001-Use-SMgrRelation-instead-of-SMgrRelationData-in-p.patch>
>>>> 
>>>> pgaio_io_set_target_smgr doesn’t update smgr, so, instead of “SMgrRelation”,  I think it’s better to change the type to "const SMgrRelationData *”.
>>> 
>>> Thanks for the review! Makes sense — I've updated the patch to v2.
>>> 
>>> -- 
>>> Regards,
>>> Japin Li
>>> ChengDu WenWu Information Technology Co., Ltd.
>>> 
>>> <v2-0001-Use-SMgrRelation-instead-of-SMgrRelationData-in-p.patch>
>> 
>> “const SMgrRelation *” will not work as you expected, you have to do “const SMgrRelationData *”. We want to protect the data the pointer pointing to from changing but the pointer itself.
> 
> Thanks for pointing that out!  I hadn't noticed the difference before.
> Updated as you suggested.
> 
>> 
>> Best regards,
>> --
>> Chao Li (Evan)
>> HighGo Software Co., Ltd.
>> https://www.highgo.com/
> 
> -- 
> Regards,
> Japin Li
> ChengDu WenWu Information Technology Co., Ltd.
> 
> <v3-0001-Add-the-const-qualifier-to-the-SMgrRelationData-p.patch>

V3 LGTM.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/









^ permalink  raw  reply  [nested|flat] 8+ messages in thread


end of thread, other threads:[~2026-03-16 03:14 UTC | newest]

Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-03-13 06:55 [PATCH v2] Use SMgrRelation instead of SMgrRelationData * in pgaio_io_set_target_smgr Japin Li <[email protected]>
2026-03-13 06:55 [PATCH v1] Use SMgrRelation instead of SMgrRelationData * in pgaio_io_set_target_smgr Japin Li <[email protected]>
2026-03-13 07:05 Use SMgrRelation instead of SMgrRelationData * in pgaio_io_set_target_smgr() Japin Li <[email protected]>
2026-03-13 07:29 ` Re: Use SMgrRelation instead of SMgrRelationData * in pgaio_io_set_target_smgr() Chao Li <[email protected]>
2026-03-14 03:07   ` Re: Use SMgrRelation instead of SMgrRelationData * in pgaio_io_set_target_smgr() Japin Li <[email protected]>
2026-03-16 00:50     ` Re: Use SMgrRelation instead of SMgrRelationData * in pgaio_io_set_target_smgr() Chao Li <[email protected]>
2026-03-16 02:31       ` Re: Use SMgrRelation instead of SMgrRelationData * in pgaio_io_set_target_smgr() Japin Li <[email protected]>
2026-03-16 03:14         ` Re: Use SMgrRelation instead of SMgrRelationData * in pgaio_io_set_target_smgr() Chao Li <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox