Andres Freund <andres@anarazel.de> writes:
> On 2017-03-23 21:26:19 -0400, Tom Lane wrote:
>> Hmm, I see ... but that only works in the cases where the caller of
>> ExecBuildProjectionInfo supplied a source slot, and a lot of 'em
>> don't.
> Right, the old and new code comment on that:
> * inputDesc can be NULL, but if it is not, we check to see whether simple
> * Vars in the tlist match the descriptor. It is important to provide
> * inputDesc for relation-scan plan nodes, as a cross check that the relation
> * hasn't been changed since the plan was made. At higher levels of a plan,
> * there is no need to recheck.
Ah, I'd forgotten the assumption that we only need to check this at scan
level.
>> I'd already put in the infrastructure to add ASSIGN_FOO_VAR_FIRST
>> step types. I could take it back out, but I wonder if it wouldn't be
>> smarter to keep it and remove the restriction in ExecBuildProjectionInfo.
>> Or maybe we could have ExecBuildProjectionInfo emit either
>> ASSIGN_FOO_VAR_FIRST or ASSIGN_FOO_VAR depending on whether it can prove
>> the reference safe.
> I think it's probably ok to just leave the check in, and remove those
> comments, and simplify the isSimpleVar stuff to only check if
> IsA(tle->expr, Var) && ((Var *) tle->expr)->varattno > 0)
Not sure. It's a pretty fair amount of duplicative code, once you finish
dealing with all the ExecJustFoo functions in addition to the main code
paths. At this point I'm inclined to take it back out and improve the
comments around ExecBuildProjectionInfo.
regards, tom lane