Re: [HACKERS] why not parallel seq scan for slow functions

On Sat, Jan 27, 2018 at 2:50 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Tue, Jan 2, 2018 at 6:38 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> [ new patch ]
>
> I think that grouping_planner() could benefit from a slightly more
> extensive rearrangement. With your patch applied, the order of
> operations is:
>
> 1. compute the scan/join target
> 2. apply the scan/join target to all paths in current_rel's pathlist
> 3. generate gather paths, possibly adding more stuff to current_rel's pathlist
> 4. rerun set_cheapest
> 5. apply the scan/join target, if parallel safe, to all paths in the
> current rel's partial_pathlist, for the benefit of upper planning
> steps
> 6. clear the partial pathlist if the target list is not parallel safe
>
> I at first thought this was outright broken because step #3 imposes
> the scan/join target without testing it for parallel-safety, but then
> I realized that generate_gather_paths will apply that target list by
> using apply_projection_to_path, which makes an is_parallel_safe test
> of its own. But it doesn't seem good for step 3 to test the
> parallel-safety of the target list separately for each path and then
> have grouping_planner do it one more time for the benefit of upper
> planning steps. Instead, I suggest that we try to get rid of the
> logic in apply_projection_to_path that knows about Gather and Gather
> Merge specifically. I think we can do that if grouping_planner does
> this:
>
> 1. compute the scan/join target
> 2. apply the scan/join target, if parallel safe, to all paths in the
> current rel's partial_pathlist
> 3. generate gather paths
> 4. clear the partial pathlist if the target list is not parallel safe
> 5. apply the scan/join target to all paths in current_rel's pathlist
> 6. rerun set_cheapest
>
> That seems like a considerably more logical order of operations. It
> avoids not only the expense of testing the scanjoin_target for
> parallel-safety multiple times, but the ugliness of having
> apply_projection_to_path know about Gather and Gather Merge as a
> special case.
>

If we want to get rid of Gather (Merge) checks in
apply_projection_to_path(), then we need some way to add a projection
path to the subpath of gather node even if that is capable of
projection as we do now. I think changing the order of applying
scan/join target won't address that unless we decide to do it for
every partial path. Another way could be that we handle that in
generate_gather_paths, but I think that won't be the idle place to add
projection.

If we want, we can compute the parallel-safety of scan/join target
once in grouping_planner and then pass it in apply_projection_to_path
to address your main concern.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com