Re: [HACKERS] Parallel Aggregate costs don't consider combine/serial/deserial funcs

On Tue, Apr 12, 2016 at 5:38 PM, David Rowley wrote: >>> One small point which I was a little unsure of in the attached is, >>> should the "if (aggref->aggdirectargs)" part of >>> count_agg_clauses_walker() be within the "if >>> (!context->combineStates)". I simply

Re: [HACKERS] Parallel Aggregate costs don't consider combine/serial/deserial funcs

On 13 April 2016 at 08:52, Robert Haas wrote: > On Sun, Apr 10, 2016 at 8:47 AM, David Rowley > wrote: >> I realised a few days ago that the parallel aggregate code does not >> cost for the combine, serialisation and deserialisation functions

Re: [HACKERS] Parallel Aggregate costs don't consider combine/serial/deserial funcs

On Sun, Apr 10, 2016 at 8:47 AM, David Rowley wrote: > I realised a few days ago that the parallel aggregate code does not > cost for the combine, serialisation and deserialisation functions at > all. Oops. > I've attached a patch which fixes this. I've committed

[HACKERS] Parallel Aggregate costs don't consider combine/serial/deserial funcs

Hi, I realised a few days ago that the parallel aggregate code does not cost for the combine, serialisation and deserialisation functions at all. I've attached a patch which fixes this. One small point which I was a little unsure of in the attached is, should the "if (aggref->aggdirectargs)"

Re: [HACKERS] Parallel Aggregate

Good news! On Tuesday, 22 March 2016, David Rowley wrote: > On 22 March 2016 at 02:35, Robert Haas > wrote: > > I have committed this after changing some of the comments. > > > > There might still be bugs ... but I don't see

Re: [HACKERS] Parallel Aggregate

On 22 March 2016 at 02:35, Robert Haas wrote: > I have committed this after changing some of the comments. > > There might still be bugs ... but I don't see them. And the speedups > look very impressive. > > Really nice work, David. Thanks for that, and thank you for

Re: [HACKERS] Parallel Aggregate

On Sun, Mar 20, 2016 at 7:30 PM, David Rowley wrote: > An updated patch is attached. This hopefully addresses your concerns > with the comment, and also the estimate_hashagg_tablesize() NULL > checking. I have committed this after changing some of the comments.

Re: [HACKERS] Parallel Aggregate

On Sun, Mar 20, 2016 at 11:24 PM, Alvaro Herrera wrote: > David Rowley wrote: >> On 21 March 2016 at 15:48, Alvaro Herrera wrote: >> > David Rowley wrote: >> > >> >> I've rewritten the comment to become: >> >> >> >> /* >> >> * Providing that

Re: [HACKERS] Parallel Aggregate

On 21 March 2016 at 15:48, Alvaro Herrera wrote: > David Rowley wrote: > >> I've rewritten the comment to become: >> >> /* >> * Providing that the estimated size of the hashtable does not exceed >> * work_mem, we'll generate a HashAgg Path, although if we were unable >>

Re: [HACKERS] Parallel Aggregate

David Rowley wrote: > I've rewritten the comment to become: > > /* > * Providing that the estimated size of the hashtable does not exceed > * work_mem, we'll generate a HashAgg Path, although if we were unable > * to sort above, then we'd better generate a Path, so that we at least > * have

Re: [HACKERS] Parallel Aggregate

Hi, On 03/21/2016 12:30 AM, David Rowley wrote: On 21 March 2016 at 09:47, Tomas Vondra wrote: ... I'm not sure changing the meaning of enable_hashagg like this is a good idea. It worked as a hard switch before, while with this change that would not be the case.

Re: [HACKERS] Parallel Aggregate

On 21 March 2016 at 09:47, Tomas Vondra wrote: > On 03/20/2016 09:58 AM, David Rowley wrote: >> Thank you for the reviews. The only thing I can think to mention which >> I've not already is that I designed estimate_hashagg_tablesize() to be >> reusable in various

Re: [HACKERS] Parallel Aggregate

Hi, On 03/20/2016 09:58 AM, David Rowley wrote: On 20 March 2016 at 03:19, Robert Haas wrote: On Fri, Mar 18, 2016 at 10:32 PM, David Rowley wrote: Updated patch is attached. I think this looks structurally correct now, and I think it's

Re: [HACKERS] Parallel Aggregate

On Fri, Mar 18, 2016 at 9:16 AM, David Rowley wrote: > I've attached an updated patch. This looks substantially better than earlier versions, although I haven't studied every detail of it yet. + * Partial aggregation requires that each aggregate does not have a

Re: [HACKERS] Parallel Aggregate

On 20 March 2016 at 03:19, Robert Haas wrote: > On Fri, Mar 18, 2016 at 10:32 PM, David Rowley > wrote: >> Updated patch is attached. > > I think this looks structurally correct now, and I think it's doing > the right thing as far as

Re: [HACKERS] Parallel Aggregate

On 17 March 2016 at 01:19, Amit Kapila wrote: > Few assorted comments: > > 1. > /* > + * Determine if it's possible to perform aggregation in parallel using > + * multiple worker processes. We can permit this when there's at least one > + * partial_path in input_rel,

Re: [HACKERS] Parallel Aggregate

On 19 March 2016 at 06:15, Robert Haas wrote: > On Fri, Mar 18, 2016 at 9:16 AM, David Rowley > wrote: >> I've attached an updated patch. > > This looks substantially better than earlier versions, although I > haven't studied every detail of

Re: [HACKERS] Parallel Aggregate

On 18 March 2016 at 20:25, Amit Kapila wrote: > Few more comments: > > 1. > > + if (parse->groupClause) > + path = (Path *) create_sort_path(root, > + grouped_rel, > + path, > + root->group_pathkeys, > + -1.0); > > For final path, why do you want to sort just for group by

Re: [HACKERS] Parallel Aggregate

On 17 March 2016 at 18:05, David Rowley wrote: > Updated patch attached. Please disregard 0001-Allow-aggregation-to-happen-in-parallel_2016-03-17.patch. This contained a badly thought through last minute change to how the Gather path is generated and is broken. I

Re: [HACKERS] Parallel Aggregate

On Wed, Mar 16, 2016 at 6:49 AM, David Rowley wrote: > On 16 March 2016 at 15:04, Robert Haas wrote: >> I don't think I'd be objecting if you made PartialAggref a real >> alternative to Aggref. But that's not what you've got here. A >>

Re: [HACKERS] Parallel Aggregate

On Thu, Mar 17, 2016 at 2:13 PM, James Sewell wrote: > > Hi again, > > This is probably me missing something, but is there a reason parallel > aggregate doesn't seem to ever create append nodes containing Index scans? > > SET random_page_cost TO 0.2; > SET

Re: [HACKERS] Parallel Aggregate

On 17 March 2016 at 00:57, Robert Haas wrote: > On Wed, Mar 16, 2016 at 6:49 AM, David Rowley > wrote: >> On 16 March 2016 at 15:04, Robert Haas wrote: >>> I don't think I'd be objecting if you made PartialAggref a real

Re: [HACKERS] Parallel Aggregate

On Wed, Mar 16, 2016 at 5:05 PM, David Rowley wrote: >> Cool! Why not initialize aggpartialtype always? > > Because the follow-on patch sets that to either the serialtype or the > aggtranstype, depending on if serialisation is required. Serialisation > is required

Re: [HACKERS] Parallel Aggregate

I read this a bit, as an exercise to try to follow parallel query a bit. I think the most interesting thing I have to say is that the new error messages in ExecInitExpr do not conform to our style. Probably just downcase everything except Aggref and you're done, since they're can't-happen

Re: [HACKERS] Parallel Aggregate

On 17 March 2016 at 03:47, Robert Haas wrote: > More review comments: > > /* > +* Likewise for any partial paths, although this case > is more simple as > +* we don't track the cheapest path. > +*/ > > I think

Re: [HACKERS] Parallel Aggregate

On 17 March 2016 at 01:29, Robert Haas wrote: > On Wed, Mar 16, 2016 at 8:19 AM, Amit Kapila wrote: >> Isn't it better to call it as Parallel Aggregate instead of Partial >> Aggregate. Initialy, we have kept Partial for seqscan, but later on we >>

Re: [HACKERS] Parallel Aggregate

On Fri, Mar 18, 2016 at 10:32 PM, David Rowley wrote: > Updated patch is attached. I think this looks structurally correct now, and I think it's doing the right thing as far as parallelism is concerned. I don't see any obvious problems in the rest of it, either,

Re: [HACKERS] Parallel Aggregate

On Thu, Mar 17, 2016 at 6:41 PM, David Rowley wrote: > > On 18 March 2016 at 01:22, Amit Kapila wrote: > > On Thu, Mar 17, 2016 at 10:35 AM, David Rowley > > wrote: > > Updated patch is attached. Thanks for the

Re: [HACKERS] Parallel Aggregate

On Wed, Mar 16, 2016 at 4:19 PM, David Rowley wrote: > > On 16 March 2016 at 15:04, Robert Haas wrote: > > I don't think I'd be objecting if you made PartialAggref a real > > alternative to Aggref. But that's not what you've got here. A > >

Re: [HACKERS] Parallel Aggregate

On Wed, Mar 16, 2016 at 8:19 AM, Amit Kapila wrote: > Isn't it better to call it as Parallel Aggregate instead of Partial > Aggregate. Initialy, we have kept Partial for seqscan, but later on we > changed to Parallel Seq Scan, so I am not able to think why it is better

Re: [HACKERS] Parallel Aggregate

On Wed, Mar 16, 2016 at 7:57 AM, Robert Haas wrote: > On Wed, Mar 16, 2016 at 6:49 AM, David Rowley > wrote: >> On 16 March 2016 at 15:04, Robert Haas wrote: >>> I don't think I'd be objecting if you made PartialAggref

Re: [HACKERS] Parallel Aggregate

On Thu, Mar 17, 2016 at 10:35 AM, David Rowley wrote: > > On 17 March 2016 at 01:19, Amit Kapila wrote: > > Few assorted comments: > > > > 2. > > AggPath * > > create_agg_path(PlannerInfo *root, > > @@ -2397,9 +2399,11 @@

Re: [HACKERS] Parallel Aggregate

Hi again, This is probably me missing something, but is there a reason parallel aggregate doesn't seem to ever create append nodes containing Index scans? SET random_page_cost TO 0.2; SET max_parallel_degree TO 8; postgres=# explain SELECT sum(count_i) FROM base GROUP BY view_time_day;

Re: [HACKERS] Parallel Aggregate

On 18 March 2016 at 01:22, Amit Kapila wrote: > On Thu, Mar 17, 2016 at 10:35 AM, David Rowley > wrote: >> >> On 17 March 2016 at 01:19, Amit Kapila wrote: >> > Few assorted comments: >> > >> > 2. >> > AggPath * >>

Re: [HACKERS] Parallel Aggregate

On 19 March 2016 at 05:46, Robert Haas wrote: > On Wed, Mar 16, 2016 at 5:05 PM, David Rowley > wrote: >>> Cool! Why not initialize aggpartialtype always? >> >> Because the follow-on patch sets that to either the serialtype or the >>

Re: [HACKERS] Parallel Aggregate

On 19 March 2016 at 09:53, Alvaro Herrera wrote: > I read this a bit, as an exercise to try to follow parallel query a bit. Thanks for taking a look at this. > I think the most interesting thing I have to say is that the new error > messages in ExecInitExpr do not

Re: [HACKERS] Parallel Aggregate

On 16 March 2016 at 15:04, Robert Haas wrote: > I don't think I'd be objecting if you made PartialAggref a real > alternative to Aggref. But that's not what you've got here. A > PartialAggref is just a wrapper around an underlying Aggref that > changes the interpretation

Re: [HACKERS] Parallel Aggregate

On Tue, Mar 15, 2016 at 9:23 PM, David Rowley wrote: >>> Should I update the patch to use the method you describe? >> >> Well, my feeling is that is going to make this a lot smaller and >> simpler, so I think so. But if you disagree strongly, let's discuss >>

Re: [HACKERS] Parallel Aggregate

On 16 March 2016 at 14:08, Robert Haas wrote: > On Tue, Mar 15, 2016 at 8:55 PM, David Rowley > wrote: >> On 16 March 2016 at 13:42, Robert Haas wrote: >>> On Tue, Mar 15, 2016 at 8:04 PM, David Rowley >>>

Re: [HACKERS] Parallel Aggregate

On Tue, Mar 15, 2016 at 8:55 PM, David Rowley wrote: > On 16 March 2016 at 13:42, Robert Haas wrote: >> On Tue, Mar 15, 2016 at 8:04 PM, David Rowley >> wrote: >>> On 16 March 2016 at 12:58, Robert Haas

Re: [HACKERS] Parallel Aggregate

On 16 March 2016 at 13:42, Robert Haas wrote: > On Tue, Mar 15, 2016 at 8:04 PM, David Rowley > wrote: >> On 16 March 2016 at 12:58, Robert Haas wrote: >>> ...and why would one be true and the other false? >> >> One

Re: [HACKERS] Parallel Aggregate

On Tue, Mar 15, 2016 at 8:04 PM, David Rowley wrote: > On 16 March 2016 at 12:58, Robert Haas wrote: >> On Tue, Mar 15, 2016 at 6:55 PM, David Rowley >> wrote: >>> On 16 March 2016 at 11:00, Robert Haas

Re: [HACKERS] Parallel Aggregate

On 16 March 2016 at 12:58, Robert Haas wrote: > On Tue, Mar 15, 2016 at 6:55 PM, David Rowley > wrote: >> On 16 March 2016 at 11:00, Robert Haas wrote: >>> I don't see why we would need to leave aggpartial out of the

Re: [HACKERS] Parallel Aggregate

On Tue, Mar 15, 2016 at 6:55 PM, David Rowley wrote: > On 16 March 2016 at 11:00, Robert Haas wrote: >> I don't see why we would need to leave aggpartial out of the equals() >> check. I must be missing something. > > See

Re: [HACKERS] Parallel Aggregate

On 16 March 2016 at 11:00, Robert Haas wrote: > I don't see why we would need to leave aggpartial out of the equals() > check. I must be missing something. See fix_combine_agg_expr_mutator() This piece of code: /* * Aggrefs for partial aggregates are wrapped up in a

Re: [HACKERS] Parallel Aggregate

On Tue, Mar 15, 2016 at 5:50 PM, David Rowley wrote: >> I still think that's solving the problem the wrong way. Why can't >> exprType(), when applied to the Aggref, do something like this? >> >> { >> Aggref *aref = (Aggref *) expr; >> if (aref->aggpartial)

Re: [HACKERS] Parallel Aggregate

On 16 March 2016 at 09:23, Robert Haas wrote: > On Mon, Mar 14, 2016 at 7:56 PM, David Rowley > wrote: >> A comment does explain this, but perhaps it's not good enough, so I've >> rewritten it to become. >> >> /* >> * PartialAggref >> * >>

Re: [HACKERS] Parallel Aggregate

On Mon, Mar 14, 2016 at 7:56 PM, David Rowley wrote: >> More generally, why are we inventing PartialAggref >> instead of reusing Aggref? The code comments seem to contain no >> indication as to why we shouldn't need all the same details for >> PartialAggref that we

Re: [HACKERS] Parallel Aggregate

On 15 March 2016 at 13:48, David Rowley wrote: > An updated patch will follow soon. I've attached an updated patch which addresses some of Robert's and Tomas' concerns. I've not done anything about the exprCollation() yet, as I'm still unsure of what it should do. I

Re: [HACKERS] Parallel Aggregate

On 15 March 2016 at 11:24, James Sewell wrote: > On Tuesday, 15 March 2016, Robert Haas wrote: >> >> > Does the cost of the aggregate function come into this calculation at >> > all? In PostGIS land, much smaller numbers of rows can generate

Re: [HACKERS] Parallel Aggregate

On 15 March 2016 at 11:39, Tomas Vondra wrote: > I've looked at this patch today. The patch seems quite solid, but I do have > a few minor comments (or perhaps questions, given that this is the first > time I looked at the patch). > > 1) exprCollation contains this

Re: [HACKERS] Parallel Aggregate

On 15 March 2016 at 08:53, Robert Haas wrote: > I haven't fully studied every line of this yet, but here are a few comments: > > + case T_PartialAggref: > + coll = InvalidOid; /* XXX is this correct? */ > + break; >

Re: [HACKERS] Parallel Aggregate

On Tue, Mar 15, 2016 at 9:32 AM, Robert Haas wrote: > > I kind of doubt this would work well, but somebody could write a patch > for it and try it out. OK I'll give this a go today and report back. Would the eventual plan be to use pg_proc.procost for the functions from

Re: [HACKERS] Parallel Aggregate

Hi, On 03/13/2016 10:44 PM, David Rowley wrote: On 12 March 2016 at 16:31, David Rowley wrote: I've attached an updated patch which is based on commit 7087166, things are really changing fast in the grouping path area at the moment, but hopefully the dust is

Re: [HACKERS] Parallel Aggregate

On Mon, Mar 14, 2016 at 6:24 PM, James Sewell wrote: > Any chance of getting a GUC (say min_parallel_degree) added to allow setting > the initial value of parallel_degree, then changing the small relation check > to also pass if parallel_degree > 1? > > That way you

Re: [HACKERS] Parallel Aggregate

On Tuesday, 15 March 2016, Robert Haas wrote: > > > Does the cost of the aggregate function come into this calculation at > > all? In PostGIS land, much smaller numbers of rows can generate loads > > that would be effective to parallelize (worker time much >> than > >

Re: [HACKERS] Parallel Aggregate

On Mon, Mar 14, 2016 at 3:56 PM, Paul Ramsey wrote: > On Sun, Mar 13, 2016 at 7:31 PM, David Rowley > wrote: >> On 14 March 2016 at 14:52, James Sewell wrote: >>> One question - how is the upper limit of workers

Re: [HACKERS] Parallel Aggregate

On Sun, Mar 13, 2016 at 7:31 PM, David Rowley wrote: > On 14 March 2016 at 14:52, James Sewell wrote: >> One question - how is the upper limit of workers chosen? > > See create_parallel_paths() in allpaths.c. Basically the bigger the >

Re: [HACKERS] Parallel Aggregate

On Sun, Mar 13, 2016 at 5:44 PM, David Rowley wrote: > On 12 March 2016 at 16:31, David Rowley wrote: >> I've attached an updated patch which is based on commit 7087166, >> things are really changing fast in the grouping path area at

Re: [HACKERS] Parallel Aggregate

On 14 March 2016 at 17:05, James Sewell wrote: > > Hi again, > > I've been playing around with inheritance combined with this patch. Currently > it looks like you are taking max(parallel_degree) from all the child tables > and using that for the number of workers. > >

Re: [HACKERS] Parallel Aggregate

On Mon, Mar 14, 2016 at 3:05 PM, David Rowley wrote: > > Things to try: > 1. alter table a add column ts_date date; update a set ts_date = > date_trunc('DAY',ts); vacuum full analyze ts; > 2. or, create index on a (date_trunc('DAY',ts)); analyze a; > 3. or for

Re: [HACKERS] Parallel Aggregate

Hi again, I've been playing around with inheritance combined with this patch. Currently it looks like you are taking max(parallel_degree) from all the child tables and using that for the number of workers. For large machines it makes much more sense to use sum(parallel_degree) - but I've just

Re: [HACKERS] Parallel Aggregate

On 14 March 2016 at 16:39, James Sewell wrote: > > I've been testing how this works with partitioning (which seems to be > strange, but I'll post separately about that) and something odd seems to be > going on now with the parallel triggering: > > postgres=# create

Re: [HACKERS] Parallel Aggregate

Cool, I've been testing how this works with partitioning (which seems to be strange, but I'll post separately about that) and something odd seems to be going on now with the parallel triggering: postgres=# create table a as select * from base_p2015_11; SELECT 2000 postgres=# select * from a

Re: [HACKERS] Parallel Aggregate

On 14 March 2016 at 14:52, James Sewell wrote: > One question - how is the upper limit of workers chosen? See create_parallel_paths() in allpaths.c. Basically the bigger the relation (in pages) the more workers will be allocated, up until max_parallel_degree. There is

Re: [HACKERS] Parallel Aggregate

Hi, Happy to test, really looking forward to seeing this stuff in core. The explain analyze is below: Finalize HashAggregate (cost=810142.42..810882.62 rows=59216 width=16) (actual time=2282.092..2282.202 rows=15 loops=1) Group Key: (date_trunc('DAY'::text, pageview_start_tstamp)) ->

Re: [HACKERS] Parallel Aggregate

On 14 March 2016 at 14:16, James Sewell wrote: > I've done some testing with one of my data sets in an 8VPU virtual > environment and this is looking really, really good. > > My test query is: > > SELECT pageview, sum(pageview_count) > FROM fact_agg_2015_12 > GROUP BY

Re: [HACKERS] Parallel Aggregate

Hi, I've done some testing with one of my data sets in an 8VPU virtual environment and this is looking really, really good. My test query is: SELECT pageview, sum(pageview_count) FROM fact_agg_2015_12 GROUP BY date_trunc('DAY'::text, pageview); The query returns 15 rows. The fact_agg table is

Re: [HACKERS] Parallel Aggregate

On Mon, Mar 14, 2016 at 8:44 AM, David Rowley wrote: > On 12 March 2016 at 16:31, David Rowley wrote: >> I've attached an updated patch which is based on commit 7087166, >> things are really changing fast in the grouping path area at

Re: [HACKERS] Parallel Aggregate

On 12 March 2016 at 16:31, David Rowley wrote: > I've attached an updated patch which is based on commit 7087166, > things are really changing fast in the grouping path area at the > moment, but hopefully the dust is starting to settle now. The attached patch fixes

Re: [HACKERS] Parallel Aggregate

On 11 March 2016 at 03:39, David Rowley wrote: > A couple of things which I'm not 100% happy with. > > 1. make_partialgroup_input_target() doing lookups to the syscache. > Perhaps this job can be offloaded to a new function in a more suitable > location. Ideally the

Re: [HACKERS] Parallel Aggregate

On 8 March 2016 at 11:15, David Rowley wrote: > The setrefs.c parts of the patch remain completely broken. I've not > had time to look at this again yet, sorry. Ok, so again, apologies for previously sending such a broken patch. I've since managed to free up a bit

Re: [HACKERS] Parallel Aggregate

On Thu, Mar 10, 2016 at 10:55 PM, Robert Haas wrote: > On Thu, Mar 10, 2016 at 6:42 AM, David Rowley > wrote: >> The one reason that I asked about force_parallel_mode was that I >> assumed there was some buildfarm member running somewhere that

Re: [HACKERS] Parallel Aggregate

On Thu, Mar 10, 2016 at 6:42 AM, David Rowley wrote: > Hmm, it appears I only looked as far as the enum declaration, which I > expected to have something. Perhaps I'm just not used to looking up > the manual for things relating to code. I don't mind adding some

Re: [HACKERS] Parallel Aggregate

On 9 March 2016 at 04:06, Robert Haas wrote: > On Mon, Mar 7, 2016 at 5:15 PM, David Rowley > wrote: >> 3. Nothing in create_grouping_paths() looks at the force_parallel_mode >> GUC. I had a quick look at this GUC and was a bit surprised to

Re: [HACKERS] Parallel Aggregate

On Mon, Mar 7, 2016 at 4:39 PM, Tom Lane wrote: > Haribabu Kommi writes: >> 2. Temporary fix for float aggregate types in _equalAggref because of >> a change in aggtype to trans type, otherwise the parallel aggregation >> plan failure in

Re: [HACKERS] Parallel Aggregate

On Tue, Mar 8, 2016 at 4:26 PM, David Rowley wrote: >> The first one in the list will be the cheapest; why not just look at >> that? Sorted partial paths are interesting if some subsequent path >> construction step can make use of that sort ordering, but they're >>

Re: [HACKERS] Parallel Aggregate

On 9 March 2016 at 04:06, Robert Haas wrote: > On Mon, Mar 7, 2016 at 5:15 PM, David Rowley > wrote: >> My concerns are: >> 1. Since there's no cheapest_partial_path in RelOptInfo the code is >> currently considering every partial_path for

Re: [HACKERS] Parallel Aggregate

On Mon, Mar 7, 2016 at 5:15 PM, David Rowley wrote: > My concerns are: > 1. Since there's no cheapest_partial_path in RelOptInfo the code is > currently considering every partial_path for parallel hash aggregate. > With normal aggregation we only ever use the

Re: [HACKERS] Parallel Aggregate

On 5 March 2016 at 07:25, Robert Haas wrote: > On Thu, Mar 3, 2016 at 11:00 PM, David Rowley >> 3. The code never attempts to mix and match Grouping Agg and Hash Agg >> plans. e.g it could be an idea to perform Partial Hash Aggregate -> >> Gather -> Sort -> Finalize Group

Re: [HACKERS] Parallel Aggregate

On 7 March 2016 at 18:19, Haribabu Kommi wrote: > Here I attached update patch with further changes, > 1. Explain plan changes for parallel grouping Perhaps someone might disagree with me, but I'm not all that sure I really get the need for that. With nodeAgg.c we're

Re: [HACKERS] Parallel Aggregate

Haribabu Kommi writes: > 2. Temporary fix for float aggregate types in _equalAggref because of > a change in aggtype to trans type, otherwise the parallel aggregation > plan failure in set_plan_references. whenever the aggtype is not matched, > it verifies with the trans

Re: [HACKERS] Parallel Aggregate

On Sun, Mar 6, 2016 at 10:21 PM, Haribabu Kommi wrote: > > Pending: > 1. Explain plan needs to be corrected for parallel grouping similar like > parallel aggregate. Here I attached update patch with further changes, 1. Explain plan changes for parallel grouping 2.

Re: [HACKERS] Parallel Aggregate

On Fri, Mar 4, 2016 at 3:00 PM, David Rowley wrote: > On 17 February 2016 at 17:50, Haribabu Kommi wrote: >> Here I attached a draft patch based on previous discussions. It still needs >> better comments and optimization. > > Over in [1]

Re: [HACKERS] Parallel Aggregate

On Thu, Mar 3, 2016 at 11:00 PM, David Rowley wrote: > On 17 February 2016 at 17:50, Haribabu Kommi wrote: >> Here I attached a draft patch based on previous discussions. It still needs >> better comments and optimization. > > Over in [1]

Re: [HACKERS] Parallel Aggregate

On 17 February 2016 at 17:50, Haribabu Kommi wrote: > Here I attached a draft patch based on previous discussions. It still needs > better comments and optimization. Over in [1] Tom posted a large change to the grouping planner which causes large conflict with the

Re: [HACKERS] Parallel Aggregate

On Sat, Feb 13, 2016 at 3:51 PM, Robert Haas wrote: > On Sun, Feb 7, 2016 at 8:21 PM, Haribabu Kommi > wrote:future. >> Here I attached updated patch with the corrections. > > So, what about the main patch, for parallel aggregation itself? I'm >

Re: [HACKERS] Parallel Aggregate

On Sun, Feb 7, 2016 at 8:21 PM, Haribabu Kommi wrote:future. > Here I attached updated patch with the corrections. So, what about the main patch, for parallel aggregation itself? I'm reluctant to spend too much time massaging combine functions if we don't have the

Re: [HACKERS] Parallel Aggregate

On Mon, Feb 8, 2016 at 9:01 AM, Robert Haas wrote: > On Thu, Jan 21, 2016 at 11:25 PM, Haribabu Kommi > wrote: >> [ new patch ] > > This patch contains a number of irrelevant hunks that really ought not > to be here and make the patch harder to

Re: [HACKERS] Parallel Aggregate

On Sun, Jan 24, 2016 at 7:56 PM, Haribabu Kommi wrote: > On Sat, Jan 23, 2016 at 12:59 PM, Haribabu Kommi > wrote: >> Here I attached updated patch with additional combine function for >> two stage aggregates also. > > A wrong combine function

Re: [HACKERS] Parallel Aggregate

On Thu, Jan 21, 2016 at 11:25 PM, Haribabu Kommi wrote: > [ new patch ] This patch contains a number of irrelevant hunks that really ought not to be here and make the patch harder to understand, like this: -* Generate appropriate target list

Re: [HACKERS] Parallel Aggregate

On Mon, Feb 8, 2016 at 2:00 AM, Robert Haas wrote: > On Sun, Jan 24, 2016 at 7:56 PM, Haribabu Kommi > wrote: >> On Sat, Jan 23, 2016 at 12:59 PM, Haribabu Kommi >> wrote: >>> Here I attached updated patch with

Re: [HACKERS] Parallel Aggregate

On Sat, Jan 23, 2016 at 12:59 PM, Haribabu Kommi wrote: > > Here I attached updated patch with additional combine function for > two stage aggregates also. A wrong combine function was added in pg_aggregate.h in the earlier patch that leading to initdb problem.

Re: [HACKERS] Parallel Aggregate

On 22 January 2016 at 17:25, Haribabu Kommi wrote: > Along with these changes, I added a float8 combine function to see > how it works under parallel aggregate, it is working fine for float4, but > giving small data mismatch with float8 data type. > > postgres=# select

Re: [HACKERS] Parallel Aggregate

On Fri, Jan 22, 2016 at 10:13 PM, David Rowley wrote: > On 22 January 2016 at 17:25, Haribabu Kommi wrote: >> Along with these changes, I added a float8 combine function to see >> how it works under parallel aggregate, it is working fine

Re: [HACKERS] Parallel Aggregate

On 21 January 2016 at 18:26, Haribabu Kommi wrote: > Here I attached update parallel aggregate patch on top of recent commits > of combine aggregate and parallel join patch. It still lacks of cost > comparison > code to compare both parallel and normal aggregates.

Re: [HACKERS] Parallel Aggregate

On Fri, Jan 22, 2016 at 7:44 AM, David Rowley wrote: > On 21 January 2016 at 18:26, Haribabu Kommi wrote: >> Here I attached update parallel aggregate patch on top of recent commits >> of combine aggregate and parallel join patch. It still

Re: [HACKERS] Parallel Aggregate

On Thu, Dec 24, 2015 at 5:12 AM, Robert Haas wrote: > On Mon, Dec 21, 2015 at 6:38 PM, David Rowley > wrote: >> On 22 December 2015 at 04:16, Paul Ramsey wrote: >>> >>> Shouldn’t parallel aggregate come into play

Re: [HACKERS] Parallel Aggregate

On Mon, Dec 21, 2015 at 6:38 PM, David Rowley wrote: > On 22 December 2015 at 04:16, Paul Ramsey wrote: >> >> Shouldn’t parallel aggregate come into play regardless of scan >> selectivity? > > I'd say that the costing should take into

Re: [HACKERS] Parallel Aggregate

On 22 December 2015 at 04:16, Paul Ramsey wrote: > Shouldn’t parallel aggregate come into play regardless of scan selectivity? > I'd say that the costing should take into account the estimated number of groups. The more tuples that make it into each group, the more

  1   2   >