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.
