Re: [COMMITTERS] pgsql: Create hooks to let a loadable plugin monitor (or even replace) - Mailing list pgsql-hackers

Gurjeet, do you have a patch to be applied for this?

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

Gurjeet Singh wrote:
> On 5/30/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >
> > Bruce Momjian <bruce@momjian.us> writes:
> > > Gurjeet Singh wrote:
> > >> But I did not understand the haste to commit the patch within almost
> > half an
> > >> hour of proposing the second version of the patch!!!
> >
> > > It happens some times when a patch applier has gotten as far as they can
> > > go with a patch and wants to move on, with the willingness to return to
> > > the patch if there is any additional feedback.
> >
> > Er, it was quite a bit more than half an hour; about 17 hours in fact:
> > http://archives.postgresql.org/pgsql-patches/2007-05/msg00421.php
> > http://archives.postgresql.org/pgsql-committers/2007-05/msg00315.php
> 
> 
> I was referring to these two:
> 
> http://archives.postgresql.org/pgsql-patches/2007-05/msg00431.php and
> http://archives.postgresql.org/pgsql-committers/2007-05/msg00315.php
> 
> 
> In any case this patch was just a working-out of ideas I'd proposed more
> > than a month previously, so I didn't expect it to be controversial.
> >
> > But as Bruce says, nothing is set in stone at this point.  If you have
> > suggestions for improvements, we can tweak the hooks pretty much any
> > time up till 8.3 final.
> 
> 
> This being a community effort, we would expect that.
> 
> I also wished to propose to allow the plugin to completely replace (or
> augment) the plan produced by the planner (by passing in a double-pointer of
> the plan to the plugin); but I was wary that the idea might get rejected,
> for being too radical an idea.
> 
> In the last version of the planner plugin patch, the plugins were maintained
> as a list, hence allowing for multiple post-planner-plugins to work one
> after the other (the variable PPPList); much like the layered I/O driver
> architecture of Windows' NTFS sans the guarantee of ordering between the
> plugins. To this we may add the ability to pass on the result plan of one
> plugin to the next, letting them improve the plan incrementally. Next, we
> can add string identifiers like I/O drivers to guarantee the order in which
> the plugins will be executed. But again, maybe we don't need multiple
> planners working simultaneously ATM.
> 
> As for the current patch,I had only a few cosmetic changes in mind:
> 
> The comment above planner.c:planner() says '...hook variable that lets a
> plugin get control before and after the standard planning ...'; but if we
> look at the code, we are just replacing the call to standard_planner(); we
> are not calling the plugin before and after standard_planner().
> 
> Also, another cosmetic change like reducing an 'if' as follows:
> 
> Change:
> PlannedStmt *
> planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
> {
>     PlannedStmt *result;
> 
>     if (planner_hook)
>         result = (*planner_hook) (parse, cursorOptions, boundParams);
>     else
>         result = standard_planner(parse, cursorOptions, boundParams);
>     return result;
> }
> 
> To:
> PlannedStmt *
> planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
> {
>     planner_hook_type planner_func = planner_hook ? planner_hook :
> standard_planner;
> 
>     return (*planner_func) (parse, cursorOptions, boundParams);
> }
> 
> The extra IFs only disorient a normal flow of logic. These two statements
> aren't too complicated for readability.
> 
> Best regards,
> 
> PS: We can make the code more compact (at the cost of readability) like so:
> 
> return (*(planner_hook ? planner_hook : standard_planner))(parse,
> cursorOptions, boundParams);
> -- 
> gurjeet[.singh]@EnterpriseDB.com
> singh.gurjeet@{ gmail | hotmail | yahoo }.com
> 
> 17?29'34.37"N  78?30'59.76"E - Hyderabad *
> 18?32'57.25"N  73?56'25.42"E - Pune
> 
> Sent from my BlackLaptop device

--  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. +