From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | "Hiroshi Inoue" <Inoue(at)tpf(dot)co(dot)jp> |
Cc: | "pgsql-hackers" <pgsql-hackers(at)postgreSQL(dot)org> |
Subject: | Re: [HACKERS] copyObject() ? again |
Date: | 1999-03-02 16:52:18 |
Message-ID: | 18029.920393538@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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
From | Date | Subject | |
---|---|---|---|
Next Message | Vadim Mikheev | 1999-03-02 18:16:23 | Re: [HACKERS] copyObject() ? again |
Previous Message | Stefan Diestelmann | 1999-03-02 13:56:42 | Postgres under Windows NT |