Mark ItemPointer arguments as const thoughoutly

Started by Chao Li7 months ago7 messageshackers
Jump to latest
#1Chao Li
li.evan.chao@gmail.com

Hi Hacker,

This is a follow up 991295f. I searched over the src/ and make all
ItemPointer arguments as const as much as possible.

I made clean build and no waring found. And "make check" also passes. I
will create a patch on CF to see if CI passes.

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

Attachments:

v1-0001-Mark-ItemPointer-arguments-as-const-thoughoutly.patchapplication/octet-stream; name=v1-0001-Mark-ItemPointer-arguments-as-const-thoughoutly.patchDownload+99-100
#2Chao Li
li.evan.chao@gmail.com
In reply to: Chao Li (#1)
Re: Mark ItemPointer arguments as const thoughoutly

v2 tries to fix the CI failure.

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

Attachments:

v2-0001-Mark-ItemPointer-arguments-as-const-thoughoutly.patchapplication/octet-stream; name=v2-0001-Mark-ItemPointer-arguments-as-const-thoughoutly.patchDownload+100-101
#3Chao Li
li.evan.chao@gmail.com
In reply to: Chao Li (#2)
Re: Mark ItemPointer arguments as const thoughoutly

On Sep 10, 2025, at 12:20, Chao Li <li.evan.chao@gmail.com> wrote:

v2 tries to fix the CI failure.

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

<v2-0001-Mark-ItemPointer-arguments-as-const-thoughoutly.patch>

Here is the CF patch https://commitfest.postgresql.org/patch/6046/, and the CI tests passed.

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

#4Nazir Bilal Yavuz
byavuz81@gmail.com
In reply to: Chao Li (#1)
Re: Mark ItemPointer arguments as const thoughoutly

Hi,

On Wed, 10 Sept 2025 at 05:27, Chao Li <li.evan.chao@gmail.com> wrote:

Hi Hacker,

This is a follow up 991295f. I searched over the src/ and make all ItemPointer arguments as const as much as possible.

I made clean build and no waring found. And "make check" also passes. I will create a patch on CF to see if CI passes.

If you would like to run CI yourself, you can do so by forking the
Postgres GitHub repository and enabling Cirrus CI on your fork. Once
that is done, each commit will trigger a CI run. More details are
available in the README [1]https://git.postgresql.org/cgit/postgresql.git/tree/src/tools/ci/README.

[1]: https://git.postgresql.org/cgit/postgresql.git/tree/src/tools/ci/README

--
Regards,
Nazir Bilal Yavuz
Microsoft

#5Chao Li
li.evan.chao@gmail.com
In reply to: Chao Li (#3)
Re: Mark ItemPointer arguments as const thoughoutly

On Wed, Sep 10, 2025 at 1:20 PM Chao Li <li.evan.chao@gmail.com> wrote:

On Sep 10, 2025, at 12:20, Chao Li <li.evan.chao@gmail.com> wrote:

v2 tries to fix the CI failure.

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

<v2-0001-Mark-ItemPointer-arguments-as-const-thoughoutly.patch>

Here is the CF patch https://commitfest.postgresql.org/patch/6046/, and
the CI tests passed.

Rebased again to v3.

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

Attachments:

v3-0001-Mark-ItemPointer-arguments-as-const-thoughoutly.patchapplication/octet-stream; name=v3-0001-Mark-ItemPointer-arguments-as-const-thoughoutly.patchDownload+100-101
#6Peter Eisentraut
peter_e@gmx.net
In reply to: Chao Li (#5)
Re: Mark ItemPointer arguments as const thoughoutly

On 26.09.25 04:54, Chao Li wrote:

On Wed, Sep 10, 2025 at 1:20 PM Chao Li <li.evan.chao@gmail.com
<mailto:li.evan.chao@gmail.com>> wrote:

On Sep 10, 2025, at 12:20, Chao Li <li.evan.chao@gmail.com
<mailto:li.evan.chao@gmail.com>> wrote:

v2 tries to fix the CI failure.

Chao Li (Evan)
---------------------
HighGo Software Co., Ltd.
https://www.highgo.com/ <https://www.highgo.com/&gt;

<v2-0001-Mark-ItemPointer-arguments-as-const-thoughoutly.patch>

Here is the CF patch https://commitfest.postgresql.org/patch/6046/
<https://commitfest.postgresql.org/patch/6046/&gt;, and the CI tests
passed.

I have committed most of this patch.

I didn't like the few places in itemptr.h where you changed an existing
ItemPointerData* back to ItemPointer, so I left those out.

More importantly, some of the proposed changes change the signatures of
callback functions in the index or table AM APIs. (And also the
documentation wasn't updated.) This would break source code
compatibility with existing extensions that use those APIs. There are
have been previous proposals like this in [0]/messages/by-id/14c31f4a-0347-0805-dce8-93a9072c05a5@eisentraut.org, [1]/messages/by-id/f8c739d9-f48d-4187-b214-df3391ba41ab@eisentraut.org where the changes to
those APIs were not committed. And your other patch 'Mark function
arguments of type "Datum *" as "const Datum *" where possible' might
have similar problems (although it looks like it's touching different
places than [0]/messages/by-id/14c31f4a-0347-0805-dce8-93a9072c05a5@eisentraut.org and [1]/messages/by-id/f8c739d9-f48d-4187-b214-df3391ba41ab@eisentraut.org). So I left those changes out of what I committed.

That doesn't mean we can never change these APIs. We certainly change
many backend APIs all the time, and extensions providing table or index
AMs are sophisticated and will need to make adjustments anyway. But it
would be better if we did any changes in a deliberate way with explicit
notice and advice for extensions (like commit 76acf4b722f for example).

[0]: /messages/by-id/14c31f4a-0347-0805-dce8-93a9072c05a5@eisentraut.org
/messages/by-id/14c31f4a-0347-0805-dce8-93a9072c05a5@eisentraut.org
[1]: /messages/by-id/f8c739d9-f48d-4187-b214-df3391ba41ab@eisentraut.org
/messages/by-id/f8c739d9-f48d-4187-b214-df3391ba41ab@eisentraut.org

#7Chao Li
li.evan.chao@gmail.com
In reply to: Peter Eisentraut (#6)
Re: Mark ItemPointer arguments as const thoughoutly

Hi Peter,

On Oct 30, 2025, at 21:30, Peter Eisentraut <peter@eisentraut.org> wrote:

I have committed most of this patch.

Thank you so much for committing my patch and for your guidance. I’m still ramping up on PostgreSQL development, and your guidance has been super helpful to me.

I didn't like the few places in itemptr.h where you changed an existing ItemPointerData* back to ItemPointer, so I left those out.

More importantly, some of the proposed changes change the signatures of callback functions in the index or table AM APIs. (And also the documentation wasn't updated.) This would break source code compatibility with existing extensions that use those APIs. There are have been previous proposals like this in [0], [1] where the changes to those APIs were not committed. And your other patch 'Mark function arguments of type "Datum *" as "const Datum *" where possible' might have similar problems (although it looks like it's touching different places than [0] and [1]). So I left those changes out of what I committed.

I will revisit that patch once I am back from vacation.

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