A few thoughts on the plan inval extension patch

I've been looking at
http://archives.postgresql.org/message-id/48B6709E.4090903@gmail.com
and am not very happy with it. One big problem is that it has the
parser collecting the list of functions referenced by a plan, which
is quite bogus --- consider functions introduced into the tree during
rewrite, or by means of an inlining operation. The other thing I
did not care for was adding an OID to SharedInvalCatcacheMsg. That
means bloating the size of sinval messages from four words to five,
which is not a good thing from a performance perspective.

What would make a lot more sense from the perspective of the sinval
code is to use the cache entry itempointer to identify the function
(or other object) being invalidated. That's what the cache itself
uses and what's already being shipped in inval messages. We would
have to modify the call signature for syscache callback functions
to pass them an ItemPointer, but that doesn't seem like a problem.

The main downside of this approach is that some part of the planner
would have to collect a list of TIDs of pg_proc entries for every
function used in a plan, which means an additional syscache lookup
(unless this could be piggybacked on some other existing lookup,
but I see no very good candidate). The function is almost certainly in
cache at this point, so it's not a huge penalty, but it's annoying.

One idea that I had was to ameliorate that by not bothering to look up
built-in functions (which for this purpose we could define as those with
OIDs less than 10000), on the assumption that they will never be
dropped. Certainly the vast majority of functions referenced in a
typical plan tree would be built-ins, and so this should get us down to
the point where the extra lookups aren't a problem. The subsequent
savings in list copying and comparisons in PlanCacheCallback are
attractive too.

Barring objections, I'll go see about whacking the patch into this
form.

regards, tom lane