Re: [NOVICE] Why is there a doubtful copyObject call in add_vars_to_targetlist

On Mon, Mar 13, 2017 at 3:52 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> writes:
> > On 13 March 2017 at 14:22, Neha Khatri <nehakhatri5(at)gmail(dot)com> wrote:
> >> This copyObject still exits in the current code. So I was wondering if
> the
> >> comment question still holds good and why the question there in first
> place.
> >> To make a new Var object, copyObject seem to be the right choice, then
> why
> >> the doubt?
>
> > The doubt is in the fact if copyObject() is required at all. The other
> > option being to simply reference the same object without having made a
> copy.
>
> Right. Note that the code that 5efe3121 replaced effectively made a new
> Var object using makeVar. The new code makes a new Var object using
> copyObject, so there's no actual behavioral change in that fragment, just
> fewer lines of code. But it's fair to wonder whether it wouldn't be safe
> just to link to the existing Var object. This is tied up in the planner's
> general willingness to scribble on its input data structures, so that
> linking to a pre-existing object is vulnerable to some unrelated bit of
> code deciding to scribble on that object. Ideally that wouldn't happen
> ... but cleaning it up looks like a mighty tedious bit of janitorial work,
> with uncertain payoff. So it hasn't happened in the last twenty years
> and I'm not prepared to bet that it ever will.
>

Then, should it be alright to remove the doubt itself?

Regards,
Neha