Thread: Re: [HACKERS] [Fwd: Index Advisor]

Gurjeet Singh wrote:
>
>
>         Please find attached two patches:
>
>     1) pg_post_planner_plugin-REL8_2_STABLE-v1.patch.gz
>     2) pg_index_adviser-REL8_2_STABLE-v26.patch.gz
>
>
>

Why are these patches against 8.2 rather than CVS HEAD? Is this not a
new feature? We never backport new features to the stable branches -
that's what makes them stable ;-)

cheers

andrew

I can't read a 7z file on my end.  Please email me the file and I will
put it at a URL.

---------------------------------------------------------------------------

Gurjeet Singh wrote:
> On 1/15/07, Andrew Dunstan <andrew@dunslane.net> wrote:
> >
> > Gurjeet Singh wrote:
> > >
> > >     1) pg_post_planner_plugin-REL8_2_STABLE-v1.patch.gz
> > >     2) pg_index_adviser-REL8_2_STABLE-v26.patch.gz
> > >
> > Why are these patches against 8.2 rather than CVS HEAD? Is this not a
> > new feature? We never backport new features to the stable branches -
> > that's what makes them stable ;-)
> >
> >
> Please find attached the patches ported to HEAD as of now. The patch to the
> contrib modules is the same as before; the version number has been kept but
> branch designator has been changed.
>
> 1) pg_post_planner_plugin-HEAD_20070116-v2.patch.gz
> 2) pg_index_adviser-HEAD_20070116-v26.7z
>
> Best regards,
>
> --
> gurjeet[.singh]@EnterpriseDB.com
> singh.gurjeet@{ gmail | hotmail | yahoo }.com

[ Attachment, skipping... ]

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: Have you checked our extensive FAQ?
>
>                http://www.postgresql.org/docs/faq

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Your patch has been added to the PostgreSQL unapplied patches list at:

    http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------


Gurjeet Singh wrote:
> On 1/20/07, Bruce Momjian <bruce@momjian.us> wrote:
> >
> >
> > I can't read a 7z file on my end.  Please email me the file and I will
> > put it at a URL.
> >
> >
> > ---------------------------------------------------------------------------
> >
> > Gurjeet Singh wrote:
> > > Please find attached the patches ported to HEAD as of now. The patch to
> > the
> > > contrib modules is the same as before; the version number has been kept
> > but
> > > branch designator has been changed.
> > >
> > > 1) pg_post_planner_plugin-HEAD_20070116-v2.patch.gz
> > > 2) pg_index_adviser-HEAD_20070116-v26.7z
> > >
> >
>
> I am attaching the .gz versions of both the patches, and CC'ing to -patches
> also. If it doesn't turn up on -patches even this time, then please do the
> needful.
>
> Thanks and best regards,
>
> --
> gurjeet[.singh]@EnterpriseDB.com
> singh.gurjeet@{ gmail | hotmail | yahoo }.com

[ Attachment, skipping... ]

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: Don't 'kill -9' the postmaster

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

"Gurjeet Singh" <singh.gurjeet@gmail.com> writes:
> Please find attached the latest version of the patch. It applies cleanly on
> REL8_2_STABLE.

The interface to the planner in this seems rather brute-force.  To run
a plan involving a hypothetical index, you have to make a bunch of
catalog entries, run the planner, and then roll back the transaction
to get rid of the entries.  Slow, ugly, and you still need another kluge
to keep the planner from believing the index has zero size.

It strikes me that there is a better way to do it, because 99% of the
planner does not look at the system catalog entries --- all it cares
about is the IndexOptInfo structs set up by plancat.c.  So there's not
really any need to make catalog entries at all AFAICS.  Rather, the
best thing would be a plugin hook at the end of get_relation_info()
that would have a chance to editorialize on the constructed IndexOptInfo
list (and maybe other properties of the RelOptInfo too).  You could
remove existing index entries or insert new ones.

I'm dissatisfied with the hard-wired hook into planner() also.
That doesn't provide any extensibility nor allow the index adviser
to be implemented as a loadable plugin.  I'm inclined to think it's
in the wrong place anyway; you've got thrashing around there to avoid
recursion but it's very fragile.  Having to dump the results into the
postmaster log isn't a nice user API either.  Perhaps what would be
better is a hook in EXPLAIN to call a plugin that can add more lines to
EXPLAIN's output, and is passed the original query and plan so that
it can re-call the planner with hypothetical indexes prepared for
insertion by the other hook.

            regards, tom lane

Gurjeet Singh wrote:
>     The interface etc. may not be beautiful, but it isn't ugly either! It is
> a lot better than manually creating pg_index records and inserting them into
> cache; we use index_create() API to create the index (build is deferred),
> and then 'rollback to savepoint' to undo those changes when the advisor is
> done. index_create() causes pg_depends entries too, so a 'RB to SP' is far
> much safer than going and deleting cache records manually.

My complaint was not that the API used in the code was non-optimal(which
I think was Tom's issue), but that the _user_ API was not very clean.
Not sure what to recommend, but I will think about it later.

--
  Bruce Momjian  <bruce@momjian.us>          http://momjian.us
  EnterpriseDB                               http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

On Tue, 2007-04-10 at 12:18 -0700, Gurjeet Singh wrote:

>     Also, although the whole plan-tree is available in
> get_relation_info(), but it wouldn't be the right place to scan other
> tables, for eg., for generating JOIN-INDEXes or materializing some
> intermediate joins. (sometime in the future we may support them!).

I like Tom's suggestion. We never thought actually creating the indexes
was a very good thing and I'd be happy to bury that idea for good.

Speed is definitely a consideration if we are to re-plan thousands of
SQL statements for a real workload.

>     If we don't run the planner twice, then the developer will have to
> run it manually twice, and compare the costs manually (with and
> without v-indexes); virtually impossible for lage applications and
> introduction of another human-error possibility.

AFAICS Tom hasn't referred to running twice or not, so I'm not very sure
what you're referring to, sorry. If you could answer Tom's suggestions
one by one directly underneath them it would be easier to discuss
things.

ISTM that you've done a great job, the trick is now to reach agreement
and finish this. If there is something still to discuss, it needs to be
very clearly tied back to Tom's comments so everyone can follow it, then
agree it. If there is a problem in Tom's suggestions that directly
effects the operation of the tool then we need to identify what that is.
But if those hooks would give us all we need, then lets agree it and fix
up the adviser plug-in later.

We really, really, really need this. Lots.

--
  Simon Riggs
  EnterpriseDB   http://www.enterprisedb.com