On 20 March 2016 at 03:19, Robert Haas <robertmhaas@gmail.com> wrote:
> On Fri, Mar 18, 2016 at 10:32 PM, David Rowley
> <david.rowley@2ndquadrant.com> 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, but I haven't thought
> hugely deeply about the way you are doing the costing, nor have I
> totally convinced myself that all of the PathTarget and setrefs stuff
> is correct. But I think it's probably pretty close. I'll study it
> some more next week.
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 places in planner.c, yet I've only made use of it
in create_grouping_paths(). I would imagine that it might be nice to
also modify the other places which perform a similar calculation to
use that function, but I don't think that needs to be done for this
patch... perhaps a follow-on cleanup.
-- David Rowley http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services