Re: [HACKERS] copyObject() ? again - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [HACKERS] copyObject() ? again
Date
Msg-id 18029.920393538@sss.pgh.pa.us
Whole thread Raw
In response to copyObject() ? again  ("Hiroshi Inoue" <Inoue@tpf.co.jp>)
Responses RE: [HACKERS] copyObject() ? again
List pgsql-hackers
I'm still not real happy about Hiroshi's proposed reimplementation of
copyObject.  It would make copyObject much slower and more memory-
hungry, which I think is a bad thing -- especially since we may use
copyObject more than now if we redesign memory allocation as was
discussed a few weeks ago.

I looked at the specific problem he's reporting (coredump involving
a SELECT inside a plpgsql function), and I believe I understand it;
it's a pretty straightforward order-of-operations bug in copyfuncs.c.
Specifically, CopyPlanFields() tries to construct the new plan node's
list of subplans, but you can't do that until the node-type-specific
fields have all been set, since they may contain references to subplans.
As it stands, a copied plan will have a subPlan list that is missing
some of its subplans.

A simple localized fix would be to rearrange operations inside
copyfuncs.c so that the SS_pull_subplan operation is not done until
the end of the node-type-specific routines for copying plan nodes.

More generally, however, I think that the very existence of the subPlan
list is a bug.  It violates the notion that "pointer equality shouldn't
matter" because the subPlan list *must* contain pointers to the actual
member objects of a plan node's other substructure.

Moreover the subPlan list isn't even buying us much --- it wouldn't be
that expensive to scan the substructure directly for plan-type nodes
when necessary, exactly as SS_pull_subplan does.  Arguably this would
take less time than is expended on memory allocation to build the
explicit subPlan list.  So I suggest getting rid of the subPlan list
entirely, rather than hacking on copyfuncs.

Plans also have an "initPlan" list that seems to have the same kind of
problem.  I don't really understand the difference between initPlans
and subPlans --- plannodes.h says   List       *initPlan;        /* Init Plan nodes (un-correlated expr
              * subselects) */   List       *subPlan;        /* Other SubPlan nodes */
 
which doesn't convey a whole lot to my mind.  Does anyone understand
why this distinction is made?  Is it really necessary?
        regards, tom lane


pgsql-hackers by date:

Previous
From: "Stefan Diestelmann"
Date:
Subject: Postgres under Windows NT
Next
From: Vadim Mikheev
Date:
Subject: Re: [HACKERS] copyObject() ? again