Michael Paquier wrote:
> On Sat, Jun 13, 2015 at 12:41 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> >> Michael Paquier wrote:
> >>> By the way, your patch does not compile properly and is not in-line
> >>> with the project's code format. See the updated patch attached ;)
> >
> >> ... or the conventions for allocating memory. Why not just use palloc()?
> >
> > That's hardly the fault of the proposed patch. But yeah, it seems like
> > much the best fix here is to get rid of the malloc (and strdup) calls in
> > this code in favor of using the palloc infrastructure. Even the calls
> > that *do* have manual failure checks are not compliant with our usual
> > coding standards.
>
> Hm. Regarding the code path mentioned by Bill something like the patch
> attached is enough with a memory context for the query description.
Right. Note this no longer needs the individual pfree() when aborting,
because the MemoryContextDelete would remove the whole thing. (Don't
bother resubmitting; I have already fixed it.) Will push shortly.
> Now, perhaps we could do more efforts with prodesc as well, see for
> example compile_pltcl_function for pltcl and similarly for plperl.
> Thoughts?
Right. It would simplify the code: create a memory context child of
TopTransactionContext, then compile the function, and if successful,
then MemoryContextSetParent to some longer-lived context. When the
function is invalidated, it's sufficient to delete the context and
create a new one. Creating the context as child of
TopTransactionContext allows you to avoid an explicit
MemoryContextDelete() in the elog(ERROR) cases while compiling.
I do wonder how many users this code has ...
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services