On Thu, Aug 07, 2014 at 09:39:57AM -0400, Robert Haas wrote: > On Wed, Aug 6, 2014 at 9:35 PM, Bruce Momjian <br...@momjian.us> wrote: > > On Sun, Jan 12, 2014 at 12:53:40PM -0500, Noah Misch wrote: > >> Further study revealed a defect in the patch's memory management, and I > >> have > >> not gotten around to correcting that. > > > > I talked to Noah and he can't continue on this item. Can someone else > > work on it? > > Well, it would be helpful if he could describe the defect he found, so > that the next person doesn't have to guess.
Quite right. Primarily, a leak: FreeTupleDesc() did a mere pfree() on a node tree, leaking all but the root node. Perhaps the best fix there is to give each TupleDesc a memory context and then simplify FreeTupleDesc() to a context deletion. That will tend to waste more memory, but I haven't thought of something clearly better. The patch passes a relcache-owned node tree to eval_const_expressions() via ExecPrepareExpr(). If that stands, I should add a comment to the effect that eval_const_expressions() must not modify in place the original tree or reference it from the result tree. The comment at expression_planner() implies that's already true. I know of no exceptions, but I have not audited. How reasonable is reliance on continued conformance to that rule? An alternative is to have the relcache always return a node tree copy, like it does in RelationGetIndexExpressions(). That sacrifices about 1/4 of the performance gain. -- Noah Misch EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
