Don't cast away const where possible

On 18.12.25 14:55, Bertrand Drouvot wrote:

Some functions are casting away the const qualifiers from their signatures in
local variables.

@@ -1304,8 +1304,8 @@ merge_overlapping_ranges(FmgrInfo *cmp, Oid colloid,
  static int
  compare_distances(const void *a, const void *b)
  {
-       DistanceValue *da = (DistanceValue *) a;
-       DistanceValue *db = (DistanceValue *) b;
+       const DistanceValue *da = (const DistanceValue *) a;
+       const DistanceValue *db = (const DistanceValue *) b;

I wonder if the better fix here wouldn't be to get rid of the cast.
It's not necessary, and without it the compiler would automatically warn
about qualifier mismatches. These comparison functions seem to be a
common pattern.

Hi,

On Mon, Dec 22, 2025 at 12:53:03PM +0100, Peter Eisentraut wrote:

On 18.12.25 14:55, Bertrand Drouvot wrote:

Some functions are casting away the const qualifiers from their signatures in
local variables.

@@ -1304,8 +1304,8 @@ merge_overlapping_ranges(FmgrInfo *cmp, Oid colloid,
static int
compare_distances(const void *a, const void *b)
{
-       DistanceValue *da = (DistanceValue *) a;
-       DistanceValue *db = (DistanceValue *) b;
+       const DistanceValue *da = (const DistanceValue *) a;
+       const DistanceValue *db = (const DistanceValue *) b;

I wonder if the better fix here wouldn't be to get rid of the cast. It's not
necessary, and without it the compiler would automatically warn about
qualifier mismatches.

Yeah, that looks better as it provides an extra safety check should the function
signature change.

These comparison functions seem to be a common
pattern.

Right, in the attached I applied your proposal on all those places.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On Dec 29, 2025, at 17:01, Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote:

Hi,

On Mon, Dec 22, 2025 at 12:53:03PM +0100, Peter Eisentraut wrote:

On 18.12.25 14:55, Bertrand Drouvot wrote:

Some functions are casting away the const qualifiers from their signatures in
local variables.

@@ -1304,8 +1304,8 @@ merge_overlapping_ranges(FmgrInfo *cmp, Oid colloid,
static int
compare_distances(const void *a, const void *b)
{
-       DistanceValue *da = (DistanceValue *) a;
-       DistanceValue *db = (DistanceValue *) b;
+       const DistanceValue *da = (const DistanceValue *) a;
+       const DistanceValue *db = (const DistanceValue *) b;

I wonder if the better fix here wouldn't be to get rid of the cast. It's not
necessary, and without it the compiler would automatically warn about
qualifier mismatches.

Yeah, that looks better as it provides an extra safety check should the function
signature change.

These comparison functions seem to be a common
pattern.

Right, in the attached I applied your proposal on all those places.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
<v2-0001-Don-t-cast-away-const-where-possible.patch><v2-0002-Add-const-to-read-only-TableInfo-pointers-in-pg_d.patch><v2-0003-Separate-read-and-write-pointers-in-pg_saslprep.patch>

I have similar patch at https://docs.qq.com/sheet/DR0JRQ3lPVGtCWW5q?tab=000001&amp;_t=1761030496005&amp;nlc=1 doing the exact same thing in pg_dump_sort.c.

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

Hi,

On Mon, Dec 29, 2025 at 09:01:46AM +0000, Bertrand Drouvot wrote:

Hi,

On Mon, Dec 22, 2025 at 12:53:03PM +0100, Peter Eisentraut wrote:

On 18.12.25 14:55, Bertrand Drouvot wrote:

Some functions are casting away the const qualifiers from their signatures in
local variables.

@@ -1304,8 +1304,8 @@ merge_overlapping_ranges(FmgrInfo *cmp, Oid colloid,
static int
compare_distances(const void *a, const void *b)
{
-       DistanceValue *da = (DistanceValue *) a;
-       DistanceValue *db = (DistanceValue *) b;
+       const DistanceValue *da = (const DistanceValue *) a;
+       const DistanceValue *db = (const DistanceValue *) b;

I wonder if the better fix here wouldn't be to get rid of the cast. It's not
necessary, and without it the compiler would automatically warn about
qualifier mismatches.

Yeah, that looks better as it provides an extra safety check should the function
signature change.

Out of curiosity, I searched for places where we could remove explicit casts when
assigning from void pointers (relying on implicit conversion instead), that would
lead to:

"
157 files changed, 387 insertions(+), 388 deletions(-)
"

That's not a small patch and I think that doing this work is valuable though.

We could imagine, working on say 20 files at a time and say once per month.
That would ease the review(s) and also avoid too many rebases for patches waiting
in the commitfest.

Thoughts?

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On 29.12.25 10:01, Bertrand Drouvot wrote:

On Mon, Dec 22, 2025 at 12:53:03PM +0100, Peter Eisentraut wrote:

On 18.12.25 14:55, Bertrand Drouvot wrote:

Some functions are casting away the const qualifiers from their signatures in
local variables.

@@ -1304,8 +1304,8 @@ merge_overlapping_ranges(FmgrInfo *cmp, Oid colloid,
static int
compare_distances(const void *a, const void *b)
{
-       DistanceValue *da = (DistanceValue *) a;
-       DistanceValue *db = (DistanceValue *) b;
+       const DistanceValue *da = (const DistanceValue *) a;
+       const DistanceValue *db = (const DistanceValue *) b;

I wonder if the better fix here wouldn't be to get rid of the cast. It's not
necessary, and without it the compiler would automatically warn about
qualifier mismatches.

Yeah, that looks better as it provides an extra safety check should the function
signature change.

These comparison functions seem to be a common
pattern.

Right, in the attached I applied your proposal on all those places.

I have committed patch 0003 (pg_saslprep).

For patch 0002, I don't understand the change for getRootTableInfo().
It returns tbinfo->parents[0] (possibly some levels deep), but the
parents field is not const-qualfied, so I don't understand the claim
that this fixes anything.

For patch 0001, this seems good, but I wonder why your patch catches
some cases and not some other similar ones. For example, in
src/backend/access/brin/brin_minmax_multi.c, you change
compare_distances(), but not the very similar compare_expanded_ranges()
and compare_values() nearby.

Hi,

On Mon, Jan 05, 2026 at 02:35:43PM +0100, Peter Eisentraut wrote:

On 29.12.25 10:01, Bertrand Drouvot wrote:

Right, in the attached I applied your proposal on all those places.

I have committed patch 0003 (pg_saslprep).

Thanks!

For patch 0002, I don't understand the change for getRootTableInfo(). It
returns tbinfo->parents[0] (possibly some levels deep), but the parents
field is not const-qualfied, so I don't understand the claim that this fixes
anything.

You're right, the function doesn't modify anything that its argument's pointer
members point to. If it did, that would be misleading to accept a const parameter
while modifying any of its non const pointer members data. getRootTableInfo()
is not one of those cases so PFA a new version without the getRootTableInfo()
related changes.

For patch 0001, this seems good, but I wonder why your patch catches some
cases and not some other similar ones. For example, in
src/backend/access/brin/brin_minmax_multi.c, you change compare_distances(),
but not the very similar compare_expanded_ranges() and compare_values()
nearby.

The initial patch was filtering out more complex functions that would need more
study. The idea was to look at those later on.

Now, about compare_expanded_ranges() and compare_values(), that's right that
those functions have similar patterns and could be included and their "extra"
study is simple as realizing that minval and maxval are Datum (so uint64_t),
are pass by values to FunctionCall2Coll() so that it can not modify them.

So, better to be consistent within the same file, those 2 functions have been
added in the attached.

Also I've added the changes for sort_item_compare() even this is a thin wrapper
so that the changes are consistent accross the mcv.c file too.

Now all the remaining ones reported by [1]https://github.com/bdrouvot/coccinelle_on_pg/blob/main/misc/search_const_away.cocci are in files not touched by the attached,
making it consistent on a per file basis.

Note that it does not take care at all of "nearby" places where we could remove
explicit casts when assigning from void pointers (for example the arg parameter
in compare_expanded_ranges() and compare_values()) as I think that could be
worth a dedicated project as stated in [2]/messages/by-id/aVTiCHBalaFCneYD@ip-10-97-1-34.eu-west-3.compute.internal.

[1]: https://github.com/bdrouvot/coccinelle_on_pg/blob/main/misc/search_const_away.cocci
[2]: /messages/by-id/aVTiCHBalaFCneYD@ip-10-97-1-34.eu-west-3.compute.internal

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On 05.01.26 17:18, Bertrand Drouvot wrote:

For patch 0001, this seems good, but I wonder why your patch catches some
cases and not some other similar ones. For example, in
src/backend/access/brin/brin_minmax_multi.c, you change compare_distances(),
but not the very similar compare_expanded_ranges() and compare_values()
nearby.

The initial patch was filtering out more complex functions that would need more
study. The idea was to look at those later on.

Now, about compare_expanded_ranges() and compare_values(), that's right that
those functions have similar patterns and could be included and their "extra"
study is simple as realizing that minval and maxval are Datum (so uint64_t),
are pass by values to FunctionCall2Coll() so that it can not modify them.

So, better to be consistent within the same file, those 2 functions have been
added in the attached.

Also I've added the changes for sort_item_compare() even this is a thin wrapper
so that the changes are consistent accross the mcv.c file too.

Now all the remaining ones reported by [1] are in files not touched by the attached,
making it consistent on a per file basis.

Note that it does not take care at all of "nearby" places where we could remove
explicit casts when assigning from void pointers (for example the arg parameter
in compare_expanded_ranges() and compare_values()) as I think that could be
worth a dedicated project as stated in [2].

I have committed the remaining two patches.

I had in my own work parked a few changes that were similar to the ones
in your patch 0001, so I threw them in there as well.

I have another patch that removes some -Wcast-qual warnings, by
adjusting some function APIs.

This is as far as I get without resorting to advanced tricks like
unconstify() or _Generic.

Hi,

On Mon, Feb 23, 2026 at 08:24:22AM +0100, Peter Eisentraut wrote:

I have another patch that removes some -Wcast-qual warnings, by adjusting
some function APIs.

This is as far as I get without resorting to advanced tricks like
unconstify() or _Generic.

Yeah, so the patch removes those:

jsonapi.c:2170:53: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
jsonapi.c:2171:52: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
jsonapi.c:2172:54: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
jsonapi.c:2170:53: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
jsonapi.c:2171:52: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
jsonapi.c:2172:54: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
jsonapi.c:2170:53: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
jsonapi.c:2171:52: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
jsonapi.c:2172:54: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
ginbulk.c:247:62: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
ginbulk.c:247:79: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]
parser.c:265:51: warning: cast discards ‘const’ qualifier from pointer target type [-Wcast-qual]

and looks ok.

I can see a "volatile" one though:

vacuum.c:1885:46: warning: cast discards ‘volatile’ qualifier from pointer target type [-Wcast-qual]

worth to "fix" like in the attached while at it? (It's not really a fix as it
moves the warning from vacuum.c to c.h, but that's consistent with 481018f2804).

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On 23.02.26 12:22, Bertrand Drouvot wrote:

I can see a "volatile" one though:

vacuum.c:1885:46: warning: cast discards ‘volatile’ qualifier from pointer target type [-Wcast-qual]

worth to "fix" like in the attached while at it? (It's not really a fix as it
moves the warning from vacuum.c to c.h, but that's consistent with 481018f2804).

That fix looks correct. But my patch says it's making changes only
without use unconstify and the like. To fix all -Wcast-qual warnings,
you'd need more changes like that, which I'm not currently proposing.

Hi,

On Tue, Feb 24, 2026 at 05:05:34PM +0100, Peter Eisentraut wrote:

On 23.02.26 12:22, Bertrand Drouvot wrote:

I can see a "volatile" one though:

vacuum.c:1885:46: warning: cast discards ‘volatile’ qualifier from pointer target type [-Wcast-qual]

worth to "fix" like in the attached while at it? (It's not really a fix as it
moves the warning from vacuum.c to c.h, but that's consistent with 481018f2804).

That fix looks correct.

Thanks for looking at it!

But my patch says it's making changes only without
use unconstify and the like.
To fix all -Wcast-qual warnings, you'd need
more changes like that, which I'm not currently proposing.

That's the only one "cast discards ‘volatile'" remaining outside c.h, that's
why I proposed a dedicated patch for it in passing. I'll open a dedicated thread
for the remaining volatile one.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com