Robert Haas <robertmh...@gmail.com> writes:
> Again, please have a look at the patch and see if it seems unsuitable
> to you for some reason.  I'm not confident of my ability to get all of
> this path stuff right without a bit of help from the guy who designed
> it.

I'm kind of in a bind right now because Tomas has produced an
FK-selectivity patch rewrite on schedule, and I already committed to
review that before beta2, so I better go do that first.  If you can wait
awhile I will try to help out more with parallel query.

Having said that, the main thing I noticed in your draft patch is that
you'd removed set_grouped_rel_consider_parallel() in favor of hard-wiring
this in create_grouping_paths():

+       if (input_rel->consider_parallel &&
+               !has_parallel_hazard((Node *) target->exprs, false) &&
+               !has_parallel_hazard((Node *) parse->havingQual, false))
+               grouped_rel->consider_parallel = true;

I think this is bad because it forces any external creators of
UPPERREL_GROUP_AGG to duplicate that code --- and heaven help everybody
if anyone is out of sync on whether to set the flag.  So I'd rather keep
set_grouped_rel_consider_parallel(), even if all it does is the above.
And make it global not static.  Ditto for the other upperrels.

Also, I wonder whether we could reduce that test to just the
has_parallel_hazard tests, so as to avoid the ordering dependency of
needing to create the top scan/join rel (and set its consider_parallel
flag) before we can create the UPPERREL_GROUP_AGG rel.  This would mean
putting more dependency on per-path parallel_safe flags to detect whether
you can't parallelize a given step for lack of parallel-safe input, but
I'm not sure that that's a bad thing.

                        regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to