| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Japin Li <japinli(at)hotmail(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Use SMgrRelation instead of SMgrRelationData * in pgaio_io_set_target_smgr() |
| Date: | 2026-03-16 00:50:55 |
| Message-ID: | A5A6A4A1-1AB5-414E-81CF-6BC22DDA1473@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Mar 14, 2026, at 11:07, Japin Li <japinli(at)hotmail(dot)com> wrote:
>
>
> Hi, Chao
>
> On Fri, 13 Mar 2026 at 15:29, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>>> On Mar 13, 2026, at 15:05, Japin Li <japinli(at)hotmail(dot)com> 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/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-03-16 01:00:41 | Re: Streamify more code paths |
| Previous Message | John Naylor | 2026-03-16 00:49:17 | Re: Reduce timing overhead of EXPLAIN ANALYZE using rdtsc? |