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

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

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

Gurjeet Singh wrote:
> On 5/30/07, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > Bruce Momjian <bruce(at)momjian(dot)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[(dot)singh](at)EnterpriseDB(dot)com
> singh(dot)gurjeet(at){ 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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

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