Re: [HACKERS] get_relation_stats_hook()

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Simon Riggs <simon(at)2ndquadrant(dot)com>
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-07-10 18:59:29
Message-ID: 12400.1215716369@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

Simon Riggs <simon(at)2ndquadrant(dot)com> writes:
> On Thu, 2008-06-26 at 12:50 -0400, Tom Lane wrote:
>> 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.

The hooks proposed here are completely useless --- it's impossible
to return anything except actual catalog data, because whatever is
returned is going to be subjected to ReleaseSysCache() later on.
I'd think you at least need to be able to turn that into a no-op,
or perhaps a pfree() call. Also it's clear that some calls of
examine_variable would still return regular syscache entries, so
the cleanup behavior has to be determinable on a per-call basis.

I also still think that you need to reconsider the level at which
the hook gets control. It's impossible given these hook definitions
to create "fake" data for an appendrel or sub-SELECT, which seems to
me to be an important requirement, at least till such time as the
core planner handles those cases better.

My feeling is that the hooks ought to be at more or less the same
semantic level as examine_variable and ReleaseVariableStats, and that
rather than making special-case hooks for the other couple of direct
accesses to STATRELATT, the right thing is to make sure that the
examine_variable hook will work for them too.

I do now concur that having the hook return a pg_statistic tuple
is appropriate --- after looking at the code that uses this stuff,
decoupling that representation would be more work than it's worth.

I think it might be a good idea to prepare an actual working example
of a plug-in that does something with the hook in order to verify
that you've got a useful definition.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2008-07-10 19:07:23 Re: CommitFest: how does handoff work for non-committer reviewers?
Previous Message Josh Berkus 2008-07-10 18:08:47 Re: CommitFest: how does handoff work for non-committer reviewers?

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2008-07-10 20:11:42 Re: [PATCHES] WIP: executor_hook for pg_stat_statements
Previous Message Abhijit Menon-Sen 2008-07-10 17:30:35 Re: WITH RECURSIVE updated to CVS TIP