Caching negative lookup results in typcache.c

In the pgsql-performance thread at
http://www.postgresql.org/message-id/flat/CAOR=d=3j1U_q-zf8+jUx1hkx8ps+N8pm=EUTqyFdJ5ov=+fawg(at)mail(dot)gmail(dot)com
it emerged that a substantial part of the slowdown Scott saw from 8.4 to
9.2 was an unexpected consequence of the fact that the planner now
considers hashing methods for implementing UNION. That leads to requests
to typcache.c to look up the hash opclass for a UNION'd datatype.
Which is fine, and fast, as long as there is one ... but if there is not,
each request causes a new physical probe into pg_opclass, because
typcache.c is not designed to cache negative results. Fixing that
results in about a 16% reduction in total query time on Scott's example
(for me, 432 ms to 360 ms, in a cassert-off build).

We learned years ago that it was critical for the catalog caches to
remember negative lookup results, so I'm a bit surprised that it's
taken us this long to realize that typcache.c needs to do it too.

If memory serves, the reason for not caching negative results was the
thought that opclasses might get added to a type over the course of a
session. However, that's pretty lame logic, because they could also
get dropped over the course of a session, and the existing typcache.c
code utterly fails to cope with that scenario; it could easily result
in attempts to call no-longer-existing operators/functions.

The attached proposed patch adjusts the typcache.c code so that negative
results are cacheable, and adds a syscache invalidation callback so that
all such results are forgotten whenever an update occurs in pg_opclass.
That's sufficient to cover the cases of CREATE/DROP OPERATOR CLASS, which
are the only supported scenarios for altering operator classes. I think
it is not necessary to watch for pg_amop or pg_amproc updates, because
the supported uses of ALTER OPERATOR FAMILY ADD/DROP OPERATOR/FUNCTION
are for cross-type family members, none of which are cached in typcache.

Comments?

Is it reasonable to think of back-patching this? I'm inclined to think
that performance is not a sufficient argument for doing that, because the
lossage occurs only for types that have a default btree opclass but no
default hash opclass or vice versa; and that is a fairly short list,
with no heavily-used types involved. However, failure to honor DROP
OPERATOR CLASS is clearly a bug, and that might be a good argument for
back-patching (though I've not heard any field complaints about it).

regards, tom lane