Re: WIP: Upper planner pathification

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com>, Petr Jelinek <petr(at)2ndquadrant(dot)com>, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: WIP: Upper planner pathification
Date: 2016-03-17 18:31:13
Message-ID: 5268.1458239473@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Wed, Mar 16, 2016 at 2:47 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>>> On Mon, Mar 14, 2016 at 9:21 PM, Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
>>>> So, even though we don't need to define multiple hook declarations,
>>>> I think the hook invocation is needed just after create_xxxx_paths()
>>>> for each. It will need to inform extension the context of hook
>>>> invocation, the argument list will take UpperRelationKind.

>>> That actually seems like a pretty good point. Otherwise you can't
>>> push anything from the upper rels down unless you are prepared to
>>> handle all of it.

>> I'm not exactly convinced of the use-case for that. What external
>> thing is likely to handle window functions but not aggregation,
>> for example?

> I'm not going to say that you're entirely wrong, but I think that
> attitude is a bit short-sighted.

Well, I'm prepared to yield to the extent of repeating the hook call
before each phase with an UpperRelationKind parameter to tell which phase
we're about to do. The main concern here is to avoid redundant
computation, but the hook can check the "kind" parameter and fall out
quickly if it has nothing useful to do at the current phase.

I do not, however, like the proposal to expose wflists and so forth.
Those are internal data structures in grouping_planner and I absolutely
refuse to promise that they're going to stay stable. (I had already
been thinking a couple of weeks ago about revising the activeWindows
data structure, now that it would be reasonably practical to cost out
different orders for doing the window functions in.) I think a hook
that has its own ideas about window function implementation methods
can gather its own information about the WFs without that much extra
cost, and it very probably wouldn't want exactly the same data that
create_window_paths uses today anyway.

So what I would now propose is

typedef void (*create_upper_paths_hook_type) (PlannerInfo *root,
UpperRelationKind stage,
RelOptInfo *input_rel);

and have this invoked at each stage right before we call
create_grouping_paths, create_window_paths, etc.

Also, I don't particularly see a need for a corresponding API for FDWs.
If an FDW is going to do anything in this space, it presumably has to
build up ForeignPaths for all the steps anyway. So I'd be inclined
to leave GetForeignUpperPaths as-is.

Comments?

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Joshua D. Drake 2016-03-17 19:03:42 Gendered language in source
Previous Message David Steele 2016-03-17 18:16:35 Re: Password identifiers, protocol aging and SCRAM protocol