Re: ERROR: ORDER/GROUP BY expression not found in targetlist

On Tue, Jun 14, 2016 at 4:48 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
wrote:

> On Mon, Jun 13, 2016 at 8:11 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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.
>
>
One more idea could be to have a flag (say parallel_safe) in PathTarget and
set it once we have ensured that the expressions in target doesn't contain
any parallel-restricted entry. In
create_pathtarget()/set_pathtarget_cost_width(), if the parallelmodeOK
flag is set, then we can evaluate target expressions for
parallel-restricted expressions and set the flag accordingly. Now, this
has the benefit that we don't need to evaluate the expressions of targets
for parallel-restricted clauses again and again. I think this way if the
flag is set once for final_target in grouping_planner, we don't need to
evaluate it again for other targets (grouping_target, scanjoin_target,
etc.) as those are derived from final_target. Similarly, I think we need
to set ReplOptInfo->reltarget and others as required.

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