Re: Fix for Index Advisor related hooks

On Tue, Feb 15, 2011 at 12:51 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com> writes:
> > On Tue, Feb 15, 2011 at 8:24 AM, Heikki Linnakangas <
> > heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> >> On 11.02.2011 22:44, Gurjeet Singh wrote:
> >>> One one hand get_actual_variable_range() expects that virtual indexes
> do
> >>> not
> >>> have an OID assigned, on the other hand explain_get_index_name_hook()
> is
> >>> handed just an index's OID to get its name back; IMHO these are based
> on
> >>> two
> >>> conflicting assumptions about whether a virtual index will have an OID
> >>> assigned.
>
> >> The new hook takes an index oid as argument, so I gather that you
> resolved
> >> the contradiction by deciding that fictitious indexes have OIDs. How do
> you
> >> assign those OIDs? Do fictitious indexes have entries in pg_index?
>
> > No, a fictitious index does not touch pg_index. The Index Advisor uses
> > GetNewOid(pg_class) to generate a new OID for the fictitious index.
>
> That seems like a very expensive, and lock-inducing, way of assigning a
> fictitious OID. They don't need to be globally unique.

They need to be unique for a run a session, and be distinguishable from
normal indexes so that explain_get_index_name_hook() can get a generated
name for the hypothetical index.

> I suggest you
> consider the idea I suggested back in 2007:
>
> * In this toy example we just assign all hypothetical indexes
> * OID 0, and the explain_get_index_name hook just prints
> * <hypothetical index>. In a realistic situation we'd probably
> * assume that OIDs smaller than, say, 100 are never the OID of
> * any real index, allowing us to identify one of up to 100
> * hypothetical indexes per plan. Then we'd need to save aside
> * some state data that would let the explain hooks print info
> * about the selected index.
>
> As far as the immediate problem goes, I agree that
> get_actual_variable_range is mistaken, but I think a cleaner and cheaper
> solution would be to add a bool "hypothetical" to IndexOptInfo.
>

Currently there are 2 sites interested in knowing if an index is
hypothetical:

1) explain_get_index_name_hook
2) get_actual_variable_range()

With this bool isHypothetical in place, explain_get_index_name() would
be unchanged, and the Index Advisor can use a locally generated Oid (not
from pg_class) to uniquely identify a hypothetical index.

And get_actual_variable_range() would be changed so:

- if (!OidIsValid(index->indexoid))
+ if (index->ishypothetical)

I can code submit a patch for that.

BTW, you use the term 'fictitious' in the comments, would it be better
to standardize the term used for such an index? So either the comment would
be changed to call it hypothetical, or the structure member would be changed
to isfictitious.

Regards,
--
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