Re: Fix for Index Advisor related hooks

Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
> On 18.02.2011 17:02, Gurjeet Singh wrote:
>>> On Wed, Feb 16, 2011 at 6:37 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>> Fetch the values you need and stuff 'em in the struct. Don't expect
>>>> relcache to do it for you.

>>> I also wish to make Index Advisor faster by not having to lookup this info
>>> every time a new query comes in and that's why I was trying to use the
>>> guts of IndexSupportInitialize() where it does the caching.

> Nah, I don't buy that, sounds like a premature optimization. Just
> planning a bunch of SQL statements, even if there's hundreds of them in
> the file, shouldn't take that long. And even if it does, I don't believe
> without evidence that the catalog lookups for the hypothetical indexes
> is where the time is spent.

But even more to the point, IndexSupportInitialize is simply not well
matched to the problem. It's designed to help produce a relcache entry
from a pg_index entry, and in particular to look up opfamily and input
datatype from an opclass OID. (Oh, and to produce info about index
support functions, which you certainly don't need.) But as far as I can
see, an index advisor would already *have* opfamily and input datatype,
because what it's going to be starting from is some query WHERE
condition that it thinks would be worth optimizing. What it's going to
get from looking up that operator in pg_amop is opfamily and opcintype
information. Looking up an opclass from that is unnecessary work as far
as I can see (you don't need it to put in IndexOptInfo, for sure), and
reversing the lookup afterwards is certainly pointless.

So even granted that performance is critical, you haven't made a case
why exposing IndexSupportInitialize is going to be useful.

regards, tom lane