On 3 March 2016 at 22:57, David Rowley <david.rowley@2ndquadrant.com> wrote:
> On 3 March 2016 at 18:04, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 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.
Actually I might have jumped the gun a little here with my complaint.
I think it does not matter about this as I've just coded Parallel
Group Aggregate part to reuse create_agg_path(), but left the
non-parallel version to make use of create_rollup_path(). I don't
think I want to go to the trouble of parallel grouping sets at this
stage anyway.
-- David Rowley http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services