Re: ERROR: ORDER/GROUP BY expression not found in targetlist - Mailing list pgsql-hackers

On Mon, Jun 13, 2016 at 8:11 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

>  I do
> not share your confidence that using apply_projection_to_path within
> create_grouping_paths is free of such a hazard.
>

Fair enough, let me try to explain the problem and someways to solve it in some more detail.  The main thing that got missed by me in the patch related to commit-04ae11f62 is that the partial path list of rel also needs to have a scanjoin_target. I was under assumption that create_grouping_paths will take care of assigning appropriate Path targets for the parallel paths generated by it.  If we see, create_grouping_paths() do take care of adding the appropriate path targets for the paths generated by that function.  However, it doesn't do anything for partial paths.   The patch sent by me yesterday [1] which was trying to assign partial_grouping_target to partial paths won't be the right fix, because (a) partial_grouping_target includes Aggregates (refer make_partialgroup_input_target) which we don't want for partial paths; (b) it is formed using grouping_target passed in function create_grouping_paths() whereas we need the path target formed from final_target for partial paths (as partial paths are scanjoin relations)  as is done for main path list (in grouping_planner(),  /* Forcibly apply that target to all the Paths for the scan/join rel.*/).   Now, I think we have following ways to solve it:

(a) check whether the scanjoin_target contains any parallel-restricted clause before applying the same to partial path list in grouping_planner.  However it could lead to duplicate checks in some cases for parallel-restricted clause, once in apply_projection_to_path() for main pathlist and then again before applying to partial paths.  I think we can avoid that by having an additional parameter in apply_projection_to_path() which can indicate if the check for parallel-safety is done inside that function and what is the result of same.

(b) generate the appropriate scanjoin_target for partial path list in create_grouping_paths using final_target. However I think this might lead to some duplicate code in create_grouping_paths() as we might have to some thing similar to what we have done in grouping_planner for generating such a target.  I think if we want we can pass scanjoin_target to create_grouping_paths() as well.   Again, we have to check for parallel-safety for scanjoin_target before applying it to partial paths, but we need to do that only when grouped_rel is considered parallel-safe.