On 2017-03-23 21:58:03 -0400, Tom Lane wrote: > Andres Freund <and...@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.
I'm ok with both. - Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
