Re: Identity projection

Friday, February 08, 2013 11:06 PM Tom Lane wrote:
> Amit Kapila <amit(dot)kapila(at)huawei(dot)com> writes:
>> On Friday, February 08, 2013 12:00 AM Tom Lane wrote:
>> As per my understanding, currently in code wherever Result node can be
>> avoided,
>> it calls function is_projection_capable_plan(), so we can even enhance
>> is_projection_capable_plan()
>> so that it can also verify the expressions of tlists. But for this we need
>> to change at all places
>> from where is_projection_capable_plan() is called.

> Hm. Really there's a whole dance that typically goes on, which is like

> if (!is_projection_capable_plan(result_plan))
> ....

> Perhaps we could encapsulate this whole sequence into a function called
> say assign_targetlist_to_plan(), which would have the responsibility to
> decide whether a Result node needs to be inserted.

If we want to encapsulate whole of above logic in assign_targetlist_to_plan(),
then the responsibility of new functionwill be much higher, because the code that
assigns targetlist is not same at all places.

For example
Related code in prepare_sort_from_pathkeys() is as below where it needs to append junk entry to target list.

if (!adjust_tlist_in_place && !is_projection_capable_plan(lefttree))
{ /* copy needed so we don't modify input's tlist below */

tlist = copyObject(tlist);
lefttree = (Plan *) make_result(root, tlist, NULL, lefttree);
}
/* Don't bother testing is_projection_capable_plan again */
adjust_tlist_in_place = true;
/*
* Add resjunk entry to input's tlist
*/
tle = makeTargetEntry(sortexpr,
list_length(tlist) + 1,
NULL,
true);
tlist = lappend(tlist, tle);
lefttree->targetlist = tlist; /* just in case NIL before */

Similar kind of code is there in grouping_planner for the case of activeWindows.Now we can change the code such that places where any new target entry has to be added to target list, move that part of code before calling assign_targetlist_to_plan or pass extra parameters to assign_targetlist_to_plan, so that it can accomodate all such cases. The story doesn't ends there, in some places it has to make a copy of targetlist before assigning it to plan's targetlist.

How about if just enhance the code as below:
if (!is_projection_capable_plan(result_plan) && compare_tlist_exprs(sub_tlist, result_plan->targetlist) )
{
result_plan = (Plan *) make_result(root,
sub_tlist,
NULL,
result_plan);

where the new function will be something as below:

bool
compare_tlist_exprs(List *tlist1, List *tlist2) {
ListCell *lp, *lc;
if (list_length(tlist1) != list_length(tlist2))
return false; /* tlists not same length */
forboth(lp, tlist1, lc, tlist2) {
TargetEntry *ptle = (TargetEntry *) lfirst(lp);
TargetEntry *ctle = (TargetEntry *) lfirst(lc);
if(!equal(ptle->expr,ctle->expr))
return false;
}
return true;
}

With Regards,
Amit Kapila.