Re: [HACKERS] get_relation_stats_hook()

From: Simon Riggs <simon(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, List pgsql-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: [HACKERS] get_relation_stats_hook()
Date: 2008-06-30 20:34:16
Message-ID: 1214858056.3845.514.camel@ebony.site
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches


On Thu, 2008-06-26 at 12:50 -0400, Tom Lane wrote:
> Simon Riggs <simon(at)2ndquadrant(dot)com> writes:
> >> Surely you didn't mean ALL calls. Please be more specific about what
> >> you're proposing.
>
> > The statistics relation STATRELATT is accessed in a few places in the
> > planner. Since it is in the syscache it is accessed directly from there.
> > I would like to add hooks so that stats data can come from somewhere
> > else other than the syscache for tables, just as we can already do with
> > get_relation_stats_hook().
>
> Well, defining the hooks as replacing STATRELATT lookups seems the wrong
> level of abstraction. What if you want to insert stats that can't be
> represented by the current pg_statistic definition? Even if there's no
> functional limitation, cons'ing up a dummy pg_statistic tuple seems the
> hard way to do it --- we didn't define get_relation_info_hook as working
> by supplying made-up catalog rows. Furthermore, many of the interesting
> cases that someone might want to hook into are code paths that don't
> even try to look up a pg_statistic row because they know there won't be
> one, such as two out of the three cases in examine_variable().
>
> I think you need to move up a level, and perhaps refactor some of the
> existing code to make it easier to inject made-up stats.

I've looked at ways of refactoring this, but the hooks provided here
complement the existing functionality fairly well.

We could add per column info to RelOptInfo, though toasted stats data is
always going to be difficult to handle.

There are other ways of doing this
1. replace selfuncs directly via the catalog
2. put in a hook for each selfunc at top level
3. decide an abstract API we would like to support

1 is possible now, 2 can still be done whether or not this patch is
accepted and I will likely submit a patch for that also, though 3 is too
open ended to pursue right now, IMHO.

Also, this patch can be backpatched more easily, to allow helping
current issues.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support

Attachment Content-Type Size
plan_hooks.v2.patch text/x-patch 5.9 KB
planhook.h text/x-chdr 1.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2008-06-30 20:41:12 Re: Fairly serious bug induced by latest guc enum changes
Previous Message Bruce Momjian 2008-06-30 20:29:48 Re: Fairly serious bug induced by latest guc enum changes

Browse pgsql-patches by date

  From Date Subject
Next Message Bruce Momjian 2008-06-30 22:11:02 Re: odd output in restore mode
Previous Message Bruce Momjian 2008-06-30 19:52:08 Re: [NOVICE] encoding problems