On 3 March 2016 at 18:04, Tom Lane <t...@sss.pgh.pa.us> wrote: > David Rowley <david.row...@2ndquadrant.com> writes: >> I agree that it would be good to get this in as soon as possible. I'm >> currently very close to being done with writing Parallel Aggregate on >> top of the upper planner changes. So far this version is much cleaner >> as there's less cruft added compared with the other version, > > Cool! If you come across any points where it seems like it could be > done better or more easily, that would be tremendously valuable feedback > at this stage.
Well since you mention it, I started on hash grouping and it was rather simple and clean as in create_agg_path() I just created a chain of the required paths and let create_plan() recursively build the Finalize Aggregate -> Gather -> Partial Aggregate -> Seq Scan plan. That was rather simple, and actually very nice when compared to how things are handled in today's grouping planner. When it comes to Group Aggregate I notice that you're using a RollupPath rather than an AggPath even when there's no grouping sets. This also means that create_agg_path() is only ever used for the AGG_HASHED strategy, even though the 'strategy' is passed as a parameter to that function, so it seemed prudent to me, to make sure all strategies are handled properly there. 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 nodes rather than just dealing with Paths and create_plan() and its recursive call magic. I can't quite see any blocker for not doing this, so would you object to separating out the treatment of Group Aggregate and Grouping Sets in create_grouping_paths() ? I think it would require less code overall. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
