Re: Fix for Index Advisor related hooks

On Wed, Feb 16, 2011 at 6:37 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com> writes:
> > On Wed, Feb 16, 2011 at 10:25 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> The only reason you'd need that code is if you were trying to construct
> >> a fake Relation structure, which seems unnecessary and undesirable.
>
> > The planner requires IndexOptInfo, and for the planner to choose the
> > hypothetical index we need to fill in the fwdsortop, revsortop, opfamily
> and
> > opcintype, and this is the information that IndexAdvisor populates using
> > IndexSupportInitialize() (at least until c0b5fac7 changed the function
> > signature.
>
> Yeah, and the set of stuff you need in IndexOptInfo changed again last
> week; see collations. Direct access to IndexSupportInitialize is even
> less useful today than it was a week ago. This stuff has changed many
> times before, and it will change again in the future, and exporting a
> private function that has an unrelated purpose is not going to insulate
> you from needing to deal with that.
>

I guess you are right.

>
> > What would be the best way to build an IndexOptInfo for a plain BTREE
> index
> > for different data types?
>
> Fetch the values you need and stuff 'em in the struct. Don't expect
> relcache to do it for you. The only reason relcache is involved in the
> current workflow is that we try to cache the information across queries
> in order to save on planner startup time ... but I don't think that that
> concern is nearly as pressing for something like Index Advisor. You'll
> have enough to do tracking changes in IndexOptInfo, you don't need to
> have to deal with refactorings inside relcache as well.
>

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. If Index Advisor went
on its own then we'll be implementing caching of opfamily and opcintype etc
in the contrib/ code. And I am pretty sure we can't do it any better than
what Postgres is currently doing in terms of managing that cache and
possibly invalidating it when some relevant DDL happens.

Would it be possible to somehow expose that cache managed by
LookupOpclassInfo()? I see the comments above it note that it does not
handle invalidation since there's no need for it because currently one
cannot ALTER an opclass. But I do not wish to be revisiting this problem
when that changes. IOW, when ALTER for opclass is implemented,
LookupOpclassInfo would be changed to handle invalidation and I wish to
leverage that; it might mean change in function signature, but I guess Index
Advisor will have to live with that.

Thanks,
--
gurjeet.singh
@ EnterpriseDB - The Enterprise Postgres Company
http://www.EnterpriseDB.com

singh(dot)gurjeet(at){ gmail | yahoo }.com
Twitter/Skype: singh_gurjeet

Mail sent from my BlackLaptop device