On Mon, Mar 13, 2017 at 3:52 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
David Rowley <david.rowley@2ndquadrant.com> writes: > On 13 March 2017 at 14:22, Neha Khatri <nehakhatri5@gmail.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?
