Re: Remove hardcoded hash opclass function signature exceptions

Peter Eisentraut <peter(at)eisentraut(dot)org> writes:
> On 06.09.24 21:43, Tom Lane wrote:
>> * 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.

That looks nicer. Thanks.

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

No, it isn't okay. What postgis (and the others) is doing is
equivalent to

regression=# create function myhash(bytea) returns int as 'hashvarlena' LANGUAGE 'internal' IMMUTABLE STRICT PARALLEL SAFE;
CREATE FUNCTION

After applying the v2 patch, I get

regression=# create function myhash(bytea) returns int as 'hashvarlena' LANGUAGE 'internal' IMMUTABLE STRICT PARALLEL SAFE;
ERROR: there is no built-in function named "hashvarlena"

The reason is that the fmgr_builtins table is built from
pg_proc.dat, and only names appearing in it can be used as 'internal'
function definitions. So you really can't remove the pg_proc entry.

The other thing that's made from pg_proc.dat is the list of extern
function declarations in fmgrprotos.h. That's why you had to add
those cowboy declarations inside hashfunc.c, which are both ugly
and not helpful for any external module that might wish to call those
functions at the C level.

Other than the business about removing those pg_proc entries,
I think this is good to go.

regards, tom lane