Re: Don't cast away const where possible

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] 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].

[1]: https://github.com/bdrouvot/coccinelle_on_pg/blob/main/misc/search_const_away.cocci
[2]: https://www.postgresql.org/message-id/aVTiCHBalaFCneYD%40ip-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