Re: [HACKERS] fixing consider_parallel for upper planner rels

2016-07-01 Thread Robert Haas
On Fri, Jul 1, 2016 at 10:52 AM, Tom Lane wrote: > Robert Haas writes: >> On Thu, Jun 30, 2016 at 5:54 PM, Tom Lane wrote: >>> And the point of that is what, exactly? If the only change is that >>> "some restrictions get enforced",

Re: [HACKERS] fixing consider_parallel for upper planner rels

2016-07-01 Thread Robert Haas
On Fri, Jul 1, 2016 at 11:09 AM, Tom Lane wrote: > Robert Haas writes: >> On Thu, Jun 30, 2016 at 5:54 PM, Tom Lane wrote: >>> Don't have time to re-read this right now, but maybe tomorrow or >>> Saturday. > >> OK, thanks. > >

Re: [HACKERS] fixing consider_parallel for upper planner rels

2016-07-01 Thread Tom Lane
Robert Haas writes: > On Thu, Jun 30, 2016 at 5:54 PM, Tom Lane wrote: >> Don't have time to re-read this right now, but maybe tomorrow or >> Saturday. > OK, thanks. There's still the extra-word problem here: +* If the input rel is marked

Re: [HACKERS] fixing consider_parallel for upper planner rels

2016-07-01 Thread Tom Lane
Robert Haas writes: > On Thu, Jun 30, 2016 at 5:54 PM, Tom Lane wrote: >> And the point of that is what, exactly? If the only change is that >> "some restrictions get enforced", I am not clear on why we need such >> a test mode in cases where the

Re: [HACKERS] fixing consider_parallel for upper planner rels

2016-06-30 Thread Robert Haas
On Thu, Jun 30, 2016 at 5:54 PM, Tom Lane wrote: >> That's pretty much right, but there are two conceptually separate >> things. The first is whether or not we actually attempt to spawn >> workers, and the second is whether or not we enter parallel mode. >> Entering parallel

Re: [HACKERS] fixing consider_parallel for upper planner rels

2016-06-30 Thread Robert Haas
On Thu, Jun 30, 2016 at 1:13 PM, Tom Lane wrote: > BTW, I just had another thought about reducing the cost of > has_parallel_hazard checks, to wit: you already made one pass over the > entire query to verify that there's no PARALLEL UNSAFE functions anywhere. > If that pass

Re: [HACKERS] fixing consider_parallel for upper planner rels

2016-06-30 Thread Tom Lane
Robert Haas writes: > On Thu, Jun 30, 2016 at 12:04 PM, Tom Lane wrote: >> * It seems like the initialization of root->parallelModeNeeded is still >> overcomplicated and/or underexplained. > That's pretty much right, but there are two conceptually

Re: [HACKERS] fixing consider_parallel for upper planner rels

2016-06-30 Thread Robert Haas
On Thu, Jun 30, 2016 at 12:04 PM, Tom Lane wrote: > Robert Haas writes: >> Here is a new patch addressing (I hope) the above comments and a few >> other things. > > Some more comments: > > * It seems like the initialization of root->parallelModeNeeded

Re: [HACKERS] fixing consider_parallel for upper planner rels

2016-06-30 Thread Tom Lane
BTW, I just had another thought about reducing the cost of has_parallel_hazard checks, to wit: you already made one pass over the entire query to verify that there's no PARALLEL UNSAFE functions anywhere. If that pass were to also track whether there are any PARALLEL RESTRICTED functions anywhere,

Re: [HACKERS] fixing consider_parallel for upper planner rels

2016-06-30 Thread Tom Lane
Robert Haas writes: > Here is a new patch addressing (I hope) the above comments and a few > other things. Some more comments: * It seems like the initialization of root->parallelModeNeeded is still overcomplicated and/or underexplained. Why do you not merely set it

Re: [HACKERS] fixing consider_parallel for upper planner rels

2016-06-29 Thread Robert Haas
On Mon, Jun 27, 2016 at 4:49 PM, Tom Lane wrote: > A few specific comments: > > * Can't we remove wholePlanParallelSafe altogether, in favor of just > examining best_path->parallel_safe in standard_planner? > > * In grouping_planner, I'm not exactly convinced that >

Re: [HACKERS] fixing consider_parallel for upper planner rels

2016-06-29 Thread Robert Haas
On Wed, Jun 29, 2016 at 1:26 PM, Tom Lane wrote: > Robert Haas writes: >> On Mon, Jun 27, 2016 at 6:04 PM, Tom Lane wrote: >>> Huh? The final tlist would go with the final_rel, ISTM, not the scan >>> relation. Maybe we have some

Re: [HACKERS] fixing consider_parallel for upper planner rels

2016-06-29 Thread Tom Lane
Robert Haas writes: > On Mon, Jun 27, 2016 at 6:04 PM, Tom Lane wrote: >> Huh? The final tlist would go with the final_rel, ISTM, not the scan >> relation. Maybe we have some rejiggering to do to make that true, though. > Mumble. You're right that

Re: [HACKERS] fixing consider_parallel for upper planner rels

2016-06-29 Thread Robert Haas
On Mon, Jun 27, 2016 at 6:04 PM, Tom Lane wrote: > Robert Haas writes: >> On Mon, Jun 27, 2016 at 5:28 PM, Tom Lane wrote: >>> Seems to me that it should generally be the case that consider_parallel >>> would already be clear on the

Re: [HACKERS] fixing consider_parallel for upper planner rels

2016-06-27 Thread Tom Lane
Robert Haas writes: > On Mon, Jun 27, 2016 at 5:28 PM, Tom Lane wrote: >> Seems to me that it should generally be the case that consider_parallel >> would already be clear on the parent rel if the tlist isn't parallel safe, >> and if it isn't we

Re: [HACKERS] fixing consider_parallel for upper planner rels

2016-06-27 Thread Robert Haas
On Mon, Jun 27, 2016 at 5:28 PM, Tom Lane wrote: > Robert Haas writes: >> On Mon, Jun 27, 2016 at 4:49 PM, Tom Lane wrote: >>> * Not following what you did to apply_projection_to_path, and the comment >>> therein isn't helping. > >>

Re: [HACKERS] fixing consider_parallel for upper planner rels

2016-06-27 Thread Tom Lane
Robert Haas writes: > On Mon, Jun 27, 2016 at 4:49 PM, Tom Lane wrote: >> * Not following what you did to apply_projection_to_path, and the comment >> therein isn't helping. > Gee, I wonder why not? :-) > The basic problem here is that applying a

Re: [HACKERS] fixing consider_parallel for upper planner rels

2016-06-27 Thread Robert Haas
On Mon, Jun 27, 2016 at 4:49 PM, Tom Lane wrote: > Robert Haas writes: >> On Mon, Jun 27, 2016 at 3:35 PM, Tom Lane wrote: >>> Oh, I thought you were still actively working on it. What patch do >>> you want me to review? > >> I'm

Re: [HACKERS] fixing consider_parallel for upper planner rels

2016-06-27 Thread Tom Lane
Robert Haas writes: > On Mon, Jun 27, 2016 at 3:35 PM, Tom Lane wrote: >> Oh, I thought you were still actively working on it. What patch do >> you want me to review? > I'm looking for an opinion on the WIP patch attached to: >

Re: [HACKERS] fixing consider_parallel for upper planner rels

2016-06-27 Thread Robert Haas
On Mon, Jun 27, 2016 at 3:35 PM, Tom Lane wrote: > Robert Haas writes: >> I'm not sure how to proceed here. I have asked Tom several times to >> look at the WIP patch and offer comments, but he so far has not done >> so. > > Oh, I thought you were

Re: [HACKERS] fixing consider_parallel for upper planner rels

2016-06-27 Thread Tom Lane
Robert Haas writes: > I'm not sure how to proceed here. I have asked Tom several times to > look at the WIP patch and offer comments, but he so far has not done > so. Oh, I thought you were still actively working on it. What patch do you want me to review?