I was idly trying to improve the just-added index AM amvalidate()
functions by having them verify the expected signatures (argument
and result types) of opclass support functions. opr_sanity currently
does this for btree, hash, and spgist functions, but not for other
cases; it'd be useful IMO if we had more complete coverage on that.
However, I was soon rudely reminded of the reason that opr_sanity
doesn't check GiST support functions, which is that their declarations
in pg_proc are wildly inconsistent. An example is that the strategy
number argument of GiST consistent functions is stated in the
documentation to be smallint, which is the way that many of the contrib
modules declare that, and all of the consistent functions I've looked
at use PG_GETARG_UINT16() to fetch it ... but most of the built-in
consistent functions declare the argument as integer not smallint.
(This has no impact on what happens at runtime, since the GiST core
code pays no attention to what the functions are declared as.)
There's not a lot of sanity to the declarations of union functions
either, and I've not even gotten to the other seven support functions.
I think it'd be a good idea to clean these things up, rather than weaken
the amvalidate() tests to the point where they'll accept all the existing
weirdnesses. And the documentation needs to match reality more closely
in any case.
Fixing the pg_proc entries in HEAD seems like no big deal, but some of
the errors are in contrib modules. If we wanted to be really clean
about that, we'd have to bump those modules' extension versions, which
is a pain in the rear. Since this has no user-visible impact AFAICS
(the support functions not being callable from SQL), I'm inclined to
just fix the incorrect declarations in-place. I think it's sufficient
if the contrib modules pass amvalidate checks in future, I don't feel
a need to make existing installations pass.
Any objections?
regards, tom lane
