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