Re: Remove hardcoded hash opclass function signature exceptions

On 06.09.24 21:43, Tom Lane wrote:
> Peter Eisentraut <peter(at)eisentraut(dot)org> writes:
>> hashvalidate(), which validates the signatures of support functions for
>> the hash AM, contains several hardcoded exceptions.
>> ...
>> This patch removes those exceptions by providing new support functions
>> that have the proper declared signatures. They internally share most of
>> the C code with the "wrong" functions they replace, so the behavior is
>> still the same.
>
> +1 for cleaning this up. A couple of minor nitpicks:
>
> * I don't really like the new control structure, or rather lack of
> structure, in hashvalidate. In particular the uncommented
> s/break/continue/ changes look like typos. They aren't, but can't
> you do this in a less confusing fashion? Or at least add comments
> like "continue not break because the code below the switch doesn't
> apply to this case".

Ok, I cleaned that up a bit.

> * Hand-picking OIDs as you did in pg_proc.dat is kind of passé now.
> I guess it's all right as long as nobody else does the same thing in
> the near future, but ...

Renumbered with the suggested "random" numbers.

>> Not done here, but maybe hashvarlena() and hashvarlenaextended() should
>> be removed from pg_proc.dat, since their use as opclass support
>> functions is now dubious.
>
> I wish we could get rid of those, but according to
> codesearch.debian.net, postgis and a couple of other extensions are
> relying on them. If we remove them we'll break any convenient upgrade
> path for those extensions.

Those are using the C function, which is ok. I was thinking about
removing the SQL function (from pg_proc.dat), because you can't use that
for much anymore. (You can't call it directly, and the hash AM will no
longer accept it.) I have done that in this patch version and added
some code comments around it.