Re: [HACKERS] WIP: Upper planner pathification

Robert Haas writes: > On Thu, Mar 17, 2016 at 2:31 PM, Tom Lane wrote: >> So what I would now propose is >> >> typedef void (*create_upper_paths_hook_type) (PlannerInfo *root, >> UpperRelationKind stage, >>

Re: [HACKERS] WIP: Upper planner pathification

.org > Subject: Re: [HACKERS] WIP: Upper planner pathification > > > -Original Message- > > From: pgsql-hackers-ow...@postgresql.org > > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tom Lane > > Sent: Friday, March 18, 2016 11:44 PM > > To: Kaigai

Re: [HACKERS] WIP: Upper planner pathification

On 3/22/16 7:28 AM, Michael Paquier wrote: On Mon, Mar 21, 2016 at 7:55 AM, Jim Nasby wrote: On 3/17/16 9:01 AM, Robert Haas wrote: I think that there are an awful lot of cases where extension authors haven't been able to quite do what they want to do without core

Re: [HACKERS] WIP: Upper planner pathification

On Mon, Mar 21, 2016 at 7:55 AM, Jim Nasby wrote: > On 3/17/16 9:01 AM, Robert Haas wrote: >> >> I think that >> there are an awful lot of cases where extension authors haven't been >> able to quite do what they want to do without core changes because >> they couldn't

Re: [HACKERS] WIP: Upper planner pathification

On 3/17/16 9:01 AM, Robert Haas wrote: I think that there are an awful lot of cases where extension authors haven't been able to quite do what they want to do without core changes because they couldn't get control in quite the right place; or they could do it but they had to cut-and-paste a lot

Re: Pushdown target list below gather node (WAS Re: [HACKERS] WIP: Upper planner pathification)

On Wed, Mar 16, 2016 at 12:36 PM, Tom Lane wrote: > Robert Haas writes: > > That doesn't update the cost of the subpath, which it probably needs to > > do. I wonder if this shouldn't be implemented by recursing. > > > if (IsA(path, GatherPath) &&

Re: Pushdown target list below gather node (WAS Re: [HACKERS] WIP: Upper planner pathification)

Robert Haas writes: > On Wed, Mar 16, 2016 at 2:02 PM, Tom Lane wrote: >> Looks pretty close. One point is that if we do end up using a Result >> node, then the parent GatherPath does not get charged for the Result >> node's cpu_per_tuple overhead.

Re: Pushdown target list below gather node (WAS Re: [HACKERS] WIP: Upper planner pathification)

On Wed, Mar 16, 2016 at 2:25 PM, Tom Lane wrote: > Robert Haas writes: >> On Wed, Mar 16, 2016 at 2:02 PM, Tom Lane wrote: >>> Looks pretty close. One point is that if we do end up using a Result >>> node, then the parent

Re: Pushdown target list below gather node (WAS Re: [HACKERS] WIP: Upper planner pathification)

Robert Haas writes: > On Wed, Mar 16, 2016 at 12:36 PM, Tom Lane wrote: >> And there is a larger problem with this: I'm not sure that it's >> appropriate for apply_projection_to_path to assume that the subpath is not >> shared with any other purposes.

Re: [HACKERS] WIP: Upper planner pathification

> Robert Haas writes: > > On Wed, Mar 16, 2016 at 2:47 PM, Tom Lane wrote: > >> Robert Haas writes: > >>> On Mon, Mar 14, 2016 at 9:21 PM, Kouhei Kaigai > >>> wrote: > So, even though we don't need to

Re: Pushdown target list below gather node (WAS Re: [HACKERS] WIP: Upper planner pathification)

Robert Haas writes: > OK, I've committed something along those lines. Thanks for the > advice, and feel free to whack it around if you have an idea for > improving it further - though IMHO this is good enough for 9.6. The committed patch looks fine to me. WRT your

Re: [HACKERS] WIP: Upper planner pathification

On Mon, Mar 14, 2016 at 9:21 PM, Kouhei Kaigai wrote: > So, even though we don't need to define multiple hook declarations, > I think the hook invocation is needed just after create__paths() > for each. It will need to inform extension the context of hook > invocation,

Re: Pushdown target list below gather node (WAS Re: [HACKERS] WIP: Upper planner pathification)

On Fri, Mar 18, 2016 at 10:08 AM, Tom Lane wrote: > Robert Haas writes: >> OK, I've committed something along those lines. Thanks for the >> advice, and feel free to whack it around if you have an idea for >> improving it further - though IMHO this is

Re: Pushdown target list below gather node (WAS Re: [HACKERS] WIP: Upper planner pathification)

Amit Kapila writes: > While reading above code changes, it looks like it is assuming that subpath > and subplan will always be same (as it is verifying projection capability > of subpath and attaching the tlist to subplan), but I think it is possible > that subpath and

Re: [HACKERS] WIP: Upper planner pathification

On Thu, Mar 17, 2016 at 4:22 PM, Tom Lane wrote: > Robert Haas writes: >> On Thu, Mar 17, 2016 at 2:31 PM, Tom Lane wrote: >>> Also, I don't particularly see a need for a corresponding API for FDWs. >>> If an FDW is going to do

Re: Pushdown target list below gather node (WAS Re: [HACKERS] WIP: Upper planner pathification)

Robert Haas writes: > That doesn't update the cost of the subpath, which it probably needs to > do. I wonder if this shouldn't be implemented by recursing. > if (IsA(path, GatherPath) && !has_parallel_hazard((Node *) target->exprs, > false)) >

Re: Pushdown target list below gather node (WAS Re: [HACKERS] WIP: Upper planner pathification)

On Wed, Mar 16, 2016 at 10:57 PM, Robert Haas wrote: > > On Wed, Mar 16, 2016 at 12:57 PM, Tom Lane wrote: > > Yeah, I was thinking about the same thing. The comment block above > > where you're looking would need some adjustment. > > OK, how about

Re: [HACKERS] WIP: Upper planner pathification

Kouhei Kaigai writes: > On Wed, Mar 16, 2016 at 2:47 PM, Tom Lane wrote: >> 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

Re: [HACKERS] WIP: Upper planner pathification

Robert Haas writes: > On Thu, Mar 17, 2016 at 2:31 PM, Tom Lane wrote: >> 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

Re: [HACKERS] WIP: Upper planner pathification

On Wed, Mar 16, 2016 at 2:47 PM, Tom Lane wrote: > Robert Haas writes: >> On Mon, Mar 14, 2016 at 9:21 PM, Kouhei Kaigai wrote: >>> So, even though we don't need to define multiple hook declarations, >>> I think the hook

Re: Pushdown target list below gather node (WAS Re: [HACKERS] WIP: Upper planner pathification)

On Wed, Mar 16, 2016 at 2:02 PM, Tom Lane wrote: > Robert Haas writes: >> On Wed, Mar 16, 2016 at 12:57 PM, Tom Lane wrote: >>> Yeah, I was thinking about the same thing. The comment block above >>> where you're looking would need

Re: Pushdown target list below gather node (WAS Re: [HACKERS] WIP: Upper planner pathification)

On Wed, Mar 16, 2016 at 3:09 AM, Amit Kapila wrote: > On Wed, Mar 9, 2016 at 11:58 PM, Robert Haas > wrote: > > > > On Wed, Mar 9, 2016 at 12:33 PM, Tom Lane wrote: > > > > > > Gather is a bit weird, because although it can

Re: Pushdown target list below gather node (WAS Re: [HACKERS] WIP: Upper planner pathification)

Robert Haas writes: > On Wed, Mar 16, 2016 at 12:57 PM, Tom Lane wrote: >> Yeah, I was thinking about the same thing. The comment block above >> where you're looking would need some adjustment. > OK, how about this? Looks pretty close. One point is

Re: Pushdown target list below gather node (WAS Re: [HACKERS] WIP: Upper planner pathification)

On Wed, Mar 16, 2016 at 12:57 PM, Tom Lane wrote: > Yeah, I was thinking about the same thing. The comment block above > where you're looking would need some adjustment. OK, how about this? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL

Re: [HACKERS] WIP: Upper planner pathification

> > Robert Haas writes: > > > On Mon, Mar 14, 2016 at 9:21 PM, Kouhei Kaigai > > > wrote: > > >> So, even though we don't need to define multiple hook declarations, > > >> I think the hook invocation is needed just after create__paths() > > >>

Re: [HACKERS] WIP: Upper planner pathification

> Robert Haas writes: > > On Mon, Mar 14, 2016 at 9:21 PM, Kouhei Kaigai wrote: > >> So, even though we don't need to define multiple hook declarations, > >> I think the hook invocation is needed just after create__paths() > >> for each. It will

Re: [HACKERS] WIP: Upper planner pathification

Robert Haas writes: > On Mon, Mar 14, 2016 at 9:21 PM, Kouhei Kaigai wrote: >> So, even though we don't need to define multiple hook declarations, >> I think the hook invocation is needed just after create__paths() >> for each. It will need to

Re: [HACKERS] WIP: Upper planner pathification

On Thu, Mar 17, 2016 at 2:31 PM, Tom Lane wrote: > 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,

Re: Pushdown target list below gather node (WAS Re: [HACKERS] WIP: Upper planner pathification)

On Thu, Mar 17, 2016 at 7:10 PM, Tom Lane wrote: > > Amit Kapila writes: > > While reading above code changes, it looks like it is assuming that subpath > > and subplan will always be same (as it is verifying projection capability > > of subpath and

Re: [HACKERS] WIP: Upper planner pathification

Robert Haas writes: > On Wed, Mar 16, 2016 at 2:47 PM, Tom Lane wrote: >> Robert Haas writes: >>> On Mon, Mar 14, 2016 at 9:21 PM, Kouhei Kaigai wrote: So, even though we don't need to define multiple

Re: [HACKERS] WIP: Upper planner pathification

.org > Subject: Re: [HACKERS] WIP: Upper planner pathification > > Kouhei Kaigai <kai...@ak.jp.nec.com> writes: > > On Wed, Mar 16, 2016 at 2:47 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > >> I do not, however, like the proposal to expose wflists and so forth. >

Pushdown target list below gather node (WAS Re: [HACKERS] WIP: Upper planner pathification)

On Wed, Mar 9, 2016 at 11:58 PM, Robert Haas wrote: > > On Wed, Mar 9, 2016 at 12:33 PM, Tom Lane wrote: > > > > Gather is a bit weird, because although it can project (and needs to, > > per the example of needing to compute a non-parallel-safe

Re: [HACKERS] WIP: Upper planner pathification

> -Original Message- > From: Tom Lane [mailto:t...@sss.pgh.pa.us] > Sent: Tuesday, March 15, 2016 2:04 AM > To: Petr Jelinek > Cc: Kaigai Kouhei(海外 浩平); David Rowley; Robert Haas; > pgsql-hackers@postgresql.org > Subject: Re: [HACKERS] WIP: Upper planner pathificat

Re: [HACKERS] WIP: Upper planner pathification

Robert Haas writes: > On Mon, Mar 14, 2016 at 1:37 PM, Tom Lane wrote: >> Yeah. An alternative definition that would support that would be to >> call the upper-path-providing callback for each FDW that's responsible >> for any base relation of the

Re: [HACKERS] WIP: Upper planner pathification

Robert Haas writes: > On Mon, Mar 14, 2016 at 1:37 PM, Tom Lane wrote: >> Yeah. An alternative definition that would support that would be to >> call the upper-path-providing callback for each FDW that's responsible >> for any base relation of the

Re: [HACKERS] WIP: Upper planner pathification

On Mon, Mar 14, 2016 at 1:37 PM, Tom Lane wrote: > Robert Haas writes: >> On Mon, Mar 14, 2016 at 1:04 PM, Tom Lane wrote: >>> It would be better if we invent an FDW callback that's meant to be >>> invoked at this stage, but only

Re: [HACKERS] WIP: Upper planner pathification

Robert Haas writes: > On Mon, Mar 14, 2016 at 1:04 PM, Tom Lane wrote: >> It would be better if we invent an FDW callback that's meant to be >> invoked at this stage, but only call it for FDW(s) actively involved >> in the query. I'm not sure exactly

Re: [HACKERS] WIP: Upper planner pathification

On Mon, Mar 14, 2016 at 1:04 PM, Tom Lane wrote: > Petr Jelinek writes: >> On 14/03/16 02:43, Kouhei Kaigai wrote: >>> Even though I couldn't check the new planner implementation entirely, >>> it seems to be the points below are good candidate to inject

Re: [HACKERS] WIP: Upper planner pathification

Petr Jelinek writes: > On 14/03/16 02:43, Kouhei Kaigai wrote: >> Even though I couldn't check the new planner implementation entirely, >> it seems to be the points below are good candidate to inject CustomPath >> (and potentially ForeignScan). >> >> - create_grouping_paths

Re: [HACKERS] WIP: Upper planner pathification

> -Original Message- > From: Petr Jelinek [mailto:p...@2ndquadrant.com] > Sent: Monday, March 14, 2016 12:18 PM > To: Kaigai Kouhei(海外 浩平); Tom Lane; David Rowley > Cc: Robert Haas; pgsql-hackers@postgresql.org > Subject: Re: [HACKERS] WIP: Upper planner pathification &

Re: [HACKERS] WIP: Upper planner pathification

On 14/03/16 02:43, Kouhei Kaigai wrote: > > CustomPath node is originally expected to generate various kind of plan > node, not only scan/join, and its interface is designed to support them. > For example, we can expect a CustomPath that generates "CustomSort". > > On the other hands, upper path

Re: [HACKERS] WIP: Upper planner pathification

Kohei <kai...@ak.jp.nec.com> > -Original Message- > From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tom Lane > Sent: Saturday, March 05, 2016 3:02 AM > To: David Rowley > Cc: Robert Haas; pgsql-hackers@postgresql.org > Subj

Re: [HACKERS] WIP: Upper planner pathification

Andres Freund writes: > On 2016-03-12 12:22:01 -0500, Tom Lane wrote: >> I wonder whether that's pathification per se. > If you're interested enough, I've uploaded a dump of the schema relevant > table to http://anarazel.de/t/lineitem_95_96_plan.dump.gz I haven't dug into

Re: [HACKERS] WIP: Upper planner pathification

On 2016-03-12 12:22:01 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2016-03-10 23:38:14 -0500, Tom Lane wrote: > >> I'll do it ... just send me the list. > > > After exporting make_agg, make_limit, make_sort_from_sortclauses and > > making some trivial adjustments

Re: [HACKERS] WIP: Upper planner pathification

Andres Freund writes: > On 2016-03-10 23:38:14 -0500, Tom Lane wrote: >> I'll do it ... just send me the list. > After exporting make_agg, make_limit, make_sort_from_sortclauses and > making some trivial adjustments due to the pull_var_clause changes > change, Citus' tests

Re: [HACKERS] WIP: Upper planner pathification

On 2016-03-10 23:38:14 -0500, Tom Lane wrote: > > Would you rather add back the exports or should I? > > I'll do it ... just send me the list. After exporting make_agg, make_limit, make_sort_from_sortclauses and making some trivial adjustments due to the pull_var_clause changes change, Citus'

Re: [HACKERS] WIP: Upper planner pathification

Andres Freund writes: > On 2016-03-10 15:03:41 -0500, Tom Lane wrote: >> What it encourages is having module boundaries that actually mean >> something, as well as code that can be refactored without having >> to worry about which extensions will complain about it. > I

Re: [HACKERS] WIP: Upper planner pathification

Hi, On 2016-03-10 15:03:41 -0500, Tom Lane wrote: > Andres Freund writes: > > Primarily because create_plan(), and/or its children, have to know about > > what you're doing; you can hide some, but not all, things below > > CustomScan nodes. > > And which of those things does

Re: [HACKERS] WIP: Upper planner pathification

Andres Freund writes: > On 2016-03-10 14:16:03 -0500, Tom Lane wrote: >> I don't deny that you *could* continue to do things that way, but >> I dispute that it's a good idea. Why can't you generate a Path tree >> and then ask create_plan() to convert it? > Primarily because

Re: [HACKERS] WIP: Upper planner pathification

On Thu, Mar 10, 2016 at 8:45 PM, Robert Haas wrote: > On Thu, Mar 10, 2016 at 2:41 PM, Andres Freund wrote: > > ISTM, that there's good enough reasons to go either way; I don't see > > what we're gaining by making these private. That just encourages >

Re: [HACKERS] WIP: Upper planner pathification

On Thu, Mar 10, 2016 at 11:45 AM, Robert Haas wrote: > On Thu, Mar 10, 2016 at 2:41 PM, Andres Freund wrote: >> ISTM, that there's good enough reasons to go either way; I don't see >> what we're gaining by making these private. That just encourages >>

Re: [HACKERS] WIP: Upper planner pathification

On Thu, Mar 10, 2016 at 2:41 PM, Andres Freund wrote: > ISTM, that there's good enough reasons to go either way; I don't see > what we're gaining by making these private. That just encourages > copy-paste coding. +1. Frustrating Citus's attempt to open-source their stuff is

Re: [HACKERS] WIP: Upper planner pathification

On 2016-03-10 14:16:03 -0500, Tom Lane wrote: > Andres Freund writes: > > In Citus' case a full PlannedStmt is generated on the master node, to > > combine the data generated on worker nodes (where the bog standard > > postgres planner is used). It's not the only way to do

Re: [HACKERS] WIP: Upper planner pathification

Andres Freund writes: > On 2016-03-10 13:48:31 -0500, Tom Lane wrote: >> That was intentional: in my opinion, nothing outside createplan.c ought >> to be making Plan nodes anymore. The expectation is that you make a >> Path describing what you want. Can you explain why, in

Re: [HACKERS] WIP: Upper planner pathification

On 2016-03-10 13:48:31 -0500, Tom Lane wrote: > Andres Freund writes: > > I see that you made a lot of formerly externally visible make_* routines > > static. The Citus extension (which will be open source in a few days) > > uses several of these (make_agg,

Re: [HACKERS] WIP: Upper planner pathification

Andres Freund writes: > I see that you made a lot of formerly externally visible make_* routines > static. The Citus extension (which will be open source in a few days) > uses several of these (make_agg, make_sort_from_sortclauses, make_limit > at least). > Can we please

Re: [HACKERS] WIP: Upper planner pathification

Hi Tom, On 2016-02-28 15:03:28 -0500, Tom Lane wrote: > diff --git a/src/include/optimizer/planmain.h > b/src/include/optimizer/planmain.h > index eaa642b..cd7338a 100644 > *** a/src/include/optimizer/planmain.h > --- b/src/include/optimizer/planmain.h > *** extern RelOptInfo

Re: [HACKERS] WIP: Upper planner pathification

On Wed, Mar 9, 2016 at 5:47 PM, Tom Lane wrote: > Alexander Korotkov writes: > > I have a question about Sort path. AFAICS this question wasn't mentioned > in > > the upthread discussion. > > We're producing Sort plans in two ways: from explicit

Re: [HACKERS] WIP: Upper planner pathification

On Wed, Mar 9, 2016 at 12:33 PM, Tom Lane wrote: > Alvaro Herrera writes: >> Tom Lane wrote: >>> Yeah, fixed. I had assumed that the existing coding in create_gather_plan >>> was OK, because it looked like it was right for a non-projecting node. >>>

Re: [HACKERS] WIP: Upper planner pathification

Alvaro Herrera writes: > Tom Lane wrote: >> Yeah, fixed. I had assumed that the existing coding in create_gather_plan >> was OK, because it looked like it was right for a non-projecting node. >> But actually Gather can project (why though?), so it's not right. > This

Re: [HACKERS] WIP: Upper planner pathification

On Wed, Mar 9, 2016 at 12:07 PM, Alvaro Herrera wrote: > Tom Lane wrote: >> Amit Kapila writes: > >> > Without setting max_parallel_degree, it works fine and generate the >> > appropriate results. Here the issue seems to be that the code in >>

Re: [HACKERS] WIP: Upper planner pathification

Tom Lane wrote: > Amit Kapila writes: > > Without setting max_parallel_degree, it works fine and generate the > > appropriate results. Here the issue seems to be that the code in > > grouping_planner doesn't apply the required PathTarget to Path below Gather > > Path

Re: [HACKERS] WIP: Upper planner pathification

Amit Kapila writes: > On latest commit-51c0f63e, I am seeing some issues w.r.t parallel query. > Consider a below case: > create table t1(c1 int, c2 char(1000)); > insert into t1 values(generate_series(1,30),''); > analyze t1; > set max_parallel_degree=2; >

Re: [HACKERS] WIP: Upper planner pathification

Alexander Korotkov writes: > I have a question about Sort path. AFAICS this question wasn't mentioned in > the upthread discussion. > We're producing Sort plans in two ways: from explicit Sort paths, and from > other paths which implicitly assumes sorting (like

Re: [HACKERS] WIP: Upper planner pathification

On Tue, Mar 8, 2016 at 2:31 AM, Tom Lane wrote: > > I wrote: > >> Attached is a version that addresses today's concerns, and also finishes > >> filling in the loose ends I'd left before, such as documentation and > >> outfuncs.c support. I think this is in a committable state

Re: [HACKERS] WIP: Upper planner pathification

Hi, Tom! I have a question about Sort path. AFAICS this question wasn't mentioned in the upthread discussion. We're producing Sort plans in two ways: from explicit Sort paths, and from other paths which implicitly assumes sorting (like MergeJoin path). Would it be better to produce Sort plan only

Re: [HACKERS] WIP: Upper planner pathification

I wrote: > There might be some other things we could do to provide a fast-path for > particularly trivial cases. I wanted to look into that before the code or tests had drifted far enough to make comparisons dubious. Attached is a simple patch that lets grouping_planner fall out with a minimum

Re: [HACKERS] WIP: Upper planner pathification

On 8 March 2016 at 10:01, Tom Lane wrote: > I've pushed it now; we'll soon see if the buildfarm finds any problems. Fantastic! I'm looking forward to all the future optimisation opportunities that this opens up. Thanks for making this happen. -- David Rowley

Re: [HACKERS] WIP: Upper planner pathification

I wrote: >> Attached is a version that addresses today's concerns, and also finishes >> filling in the loose ends I'd left before, such as documentation and >> outfuncs.c support. I think this is in a committable state now, though >> I plan to read through the whole thing again. The extra

Re: [HACKERS] WIP: Upper planner pathification

On Mon, Mar 7, 2016 at 11:09 AM, Tom Lane wrote: > Robert Haas writes: >> The currently-committed code generates paths where nested loops and >> hash joins get pushed beneath the Gather node, but does not generate >> paths where merge joins have been

Re: [HACKERS] WIP: Upper planner pathification

On Mon, Mar 7, 2016 at 3:37 PM, Robert Haas wrote: > > The currently-committed code generates paths where nested loops and > hash joins get pushed beneath the Gather node, but does not generate > paths where merge joins have been pushed beneath the Gather node. And > the

Re: [HACKERS] WIP: Upper planner pathification

Robert Haas writes: > The currently-committed code generates paths where nested loops and > hash joins get pushed beneath the Gather node, but does not generate > paths where merge joins have been pushed beneath the Gather node. And > the reason I didn't try to generate

Re: [HACKERS] WIP: Upper planner pathification

On Sun, Mar 6, 2016 at 10:32 AM, Tom Lane wrote: > Amit Kapila writes: >> On Sat, Mar 5, 2016 at 10:11 PM, Tom Lane wrote: >>> Is there some reason why hash and nestloop are safe but merge isn't? > >> I think it is because we

Re: [HACKERS] WIP: Upper planner pathification

On Mon, Mar 7, 2016 at 11:52 AM, Peter Geoghegan wrote: > > On Sun, Mar 6, 2016 at 9:59 PM, Tom Lane wrote: > > Perhaps it was intentional when written, but if Robert's advice is correct > > that the new upper-planner path nodes should copy up parallel_degree

Re: [HACKERS] WIP: Upper planner pathification

Peter Geoghegan writes: > In summary, I think it's surprising that a max_parallel_degree of 1 > doesn't disable parallel workers entirely. Yeah, it's not exactly clear what "1" should mean that's different from "0", in this case. regards, tom lane --

Re: [HACKERS] WIP: Upper planner pathification

On Sun, Mar 6, 2016 at 9:59 PM, Tom Lane wrote: > Perhaps it was intentional when written, but if Robert's advice is correct > that the new upper-planner path nodes should copy up parallel_degree from > their children, then it cannot be the case that parallel_degree>0 in a >

Re: [HACKERS] WIP: Upper planner pathification

Amit Kapila writes: > On Mon, Mar 7, 2016 at 9:13 AM, Tom Lane wrote: >> AFAICS, those are about generating partial paths, which is a completely >> different thing from whether a regular path is parallel-safe or not. > Okay, but the main point I

Re: [HACKERS] WIP: Upper planner pathification

On Mon, Mar 7, 2016 at 9:13 AM, Tom Lane wrote: > > Amit Kapila writes: > Is there some reason why hash and nestloop are safe but merge isn't? > > > To make hash and nestloop work in parallel queries, we just push those > > nodes below gather

Re: [HACKERS] WIP: Upper planner pathification

Amit Kapila writes: Is there some reason why hash and nestloop are safe but merge isn't? > To make hash and nestloop work in parallel queries, we just push those > nodes below gather node. Refer code > paths match_unsorted_outer()->consider_parallel_nestloop() >

Re: [HACKERS] WIP: Upper planner pathification

On Sun, Mar 6, 2016 at 9:02 PM, Tom Lane wrote: > > Amit Kapila writes: > > On Sat, Mar 5, 2016 at 10:11 PM, Tom Lane wrote: > >> Is there some reason why hash and nestloop are safe but merge isn't? > > > I think it is because we

Re: [HACKERS] WIP: Upper planner pathification

Amit Kapila writes: > On Sat, Mar 5, 2016 at 10:11 PM, Tom Lane wrote: >> Is there some reason why hash and nestloop are safe but merge isn't? > I think it is because we consider to pushdown hash and nestloop to workers, > but not merge join and the

Re: [HACKERS] WIP: Upper planner pathification

On Sat, Mar 5, 2016 at 10:11 PM, Tom Lane wrote: > > Amit Kapila writes: > > On Fri, Mar 4, 2016 at 11:31 PM, Tom Lane wrote: > >> (BTW, I found what seemed to be a couple of pre-existing bugs of > >> the same kind, eg

Re: [HACKERS] WIP: Upper planner pathification

I wrote: > Attached is a version that addresses today's concerns, and also finishes > filling in the loose ends I'd left before, such as documentation and > outfuncs.c support. I think this is in a committable state now, though > I plan to read through the whole thing again. Sigh, forgot to

Re: [HACKERS] WIP: Upper planner pathification

Attached is a version that addresses today's concerns, and also finishes filling in the loose ends I'd left before, such as documentation and outfuncs.c support. I think this is in a committable state now, though I plan to read through the whole thing again. regards, tom

Re: [HACKERS] WIP: Upper planner pathification

I wrote: > Amit Kapila writes: >> I think here we should use rel->consider_parallel to set parallel_safe as >> is done in create_mergejoin_path. > Well, the "rel" is going to be an upperrel that will have been > manufactured by fetch_upper_rel, and it will contain no

Re: [HACKERS] WIP: Upper planner pathification

Greg Stark writes: > What query is that lone data point that took 8ms instead of 6ms to > plan in both charts (assuming it's the same data point)? Ah, sorry, I should probably have spent a little more time on making those charts. That thing you're looking at isn't a data point,

Re: [HACKERS] WIP: Upper planner pathification

On Sat, Mar 5, 2016 at 6:09 PM, Tom Lane wrote: > There might be some other things we could do to provide a fast-path for > particularly trivial cases. But on the whole I think this plot shows that > there's no systematic problem, and indeed not really a lot of change at >

Re: [HACKERS] WIP: Upper planner pathification

I wrote: > Robert Haas writes: >> One idea might be to run a whole bunch of queries and record all of >> the planning times, and then run them all again and compare somehow. >> Maybe the regression tests, for example. > That sounds like something we could do pretty easily,

Re: [HACKERS] WIP: Upper planner pathification

Amit Kapila writes: > On Fri, Mar 4, 2016 at 11:31 PM, Tom Lane wrote: >> (BTW, I found what seemed to be a couple of pre-existing bugs of >> the same kind, eg create_mergejoin_path was different from the >> other two kinds of join as to setting

Re: [HACKERS] WIP: Upper planner pathification

On Sat, Mar 5, 2016 at 4:51 PM, Amit Kapila wrote: > > On Fri, Mar 4, 2016 at 11:31 PM, Tom Lane wrote: > > > > OK, here is a version that I think addresses all of the recent comments: > > > > * Fixed handling of parallel-query fields in new path node

Re: [HACKERS] WIP: Upper planner pathification

On Fri, Mar 4, 2016 at 11:31 PM, Tom Lane wrote: > > OK, here is a version that I think addresses all of the recent comments: > > * Fixed handling of parallel-query fields in new path node types. > (BTW, I found what seemed to be a couple of pre-existing bugs of > the same

Re: [HACKERS] WIP: Upper planner pathification

OK, here is a version that I think addresses all of the recent comments: * I refactored the grouping-sets stuff as suggested by Robert and David. The GroupingSetsPath code is now used *only* when there are grouping sets, otherwise what you get is a plain AGG_SORTED AggPath. This allowed removal

Re: [HACKERS] WIP: Upper planner pathification

On Tue, Mar 1, 2016 at 3:02 PM, Tom Lane wrote: > Well, my point is that no such path would have been generated if the > subquery hadn't had an internal reason to consider sorting on b.id. > The "accidental" part of this is that the subquery's GROUP BY key > matches what the

Re: [HACKERS] WIP: Upper planner pathification

Hi, On 03/03/2016 01:10 AM, Tom Lane wrote: Hmmm ... I'm now wondering about the "measurement error" theory. I tried to repeat this measurement locally, focusing on the select-only number since that should have a higher ratio of planning time to execution. Test setup: cassert-off build pgbench

Re: [HACKERS] WIP: Upper planner pathification

On 4 March 2016 at 09:29, Robert Haas wrote: > On Thu, Mar 3, 2016 at 2:19 PM, Tom Lane wrote: >> This leads me to the conclusion that all these new node types should >> set parallel_aware to false and copy up the other two fields from the >> child,

Re: [HACKERS] WIP: Upper planner pathification

Robert Haas writes: > On Thu, Mar 3, 2016 at 2:19 PM, Tom Lane wrote: >> Thanks. As I told Teodor last night, I can't reproduce a performance >> issue here with pgbench-style queries. Do you have any thoughts about >> how we might satisfy ourselves

Re: [HACKERS] WIP: Upper planner pathification

On Thu, Mar 3, 2016 at 2:19 PM, Tom Lane wrote: > Robert Haas writes: >> Thanks for working on this. Some review comments: > >> - I think all of the new path creation functions should bubble up >> parallel_degree from their subpath. > > Ah, thanks, I

Re: [HACKERS] WIP: Upper planner pathification

Robert Haas writes: > Thanks for working on this. Some review comments: > - I think all of the new path creation functions should bubble up > parallel_degree from their subpath. Ah, thanks, I didn't have any clue how to set that (though in my defense, the documentation

Re: [HACKERS] WIP: Upper planner pathification

On Sun, Feb 28, 2016 at 3:03 PM, Tom Lane wrote: > I'm pretty pleased with the way this turned out. grouping_planner() is > about half the length it was before, and much more straightforward IMO. > planagg.c no longer seems like a complete hack; it's a reasonable > prototype

Re: [HACKERS] WIP: Upper planner pathification

David Rowley writes: > My gripe is that I've added the required code to build the parallel > group aggregate to create_agg_path() already, but since Group > Aggregate uses the RollupPath I'm forced to add code in > create_rollup_plan() which manually stacks up Plan

  1   2   >