Amit Kapila <amit.kapil...@gmail.com> writes: > On Fri, Jul 22, 2016 at 4:32 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: >> The problem is that exec_stmt_dynexecute() loses control to the error >> thrown from the bogus command, and therefore leaks its "querystr" local >> variable --- which is not much of a leak, but it adds up if the loop >> iterates enough times. There are similar problems in many other places in >> plpgsql. Basically the issue is that while running a plpgsql function, >> CurrentMemoryContext points to a function-lifespan context (the same as >> the SPI procCxt the function is using). We also store things such as >> values of the function's variables there, so just resetting that context >> is not an option. plpgsql does have an expression-evaluation-lifespan >> context for short-lived stuff, but anything that needs to live for more >> or less the duration of a statement is put into the procedure-lifespan >> context, where it risks becoming a leak.
> Wouldn't it be better, if each nested sub-block (which is having an > exception) has a separate "SPI Proc", "SPI Exec" contexts which would > be destroyed at sub-block end (either commit or rollback)? AFAICS that would just confuse matters. In the first place, plpgsql variable values are not subtransaction-local, so they'd have to live in the outermost SPI Proc context anyway. In the second place, spi.c contains a whole lot of assumptions that actions like saving a plan are tied to the current SPI Proc/Exec contexts, so SPI-using plpgsql statements that were nested inside a BEGIN/EXCEPT would probably break: state they expect to remain valid from one execution to the next would disappear. > In short, why do you think it is better to create a new context rather > than using "SPI Exec"? "SPI Exec" has the same problem as the eval_econtext: there are already points at which it will be reset, and those can't necessarily be delayed till end of statement. In particular, _SPI_end_call will delete whatever is in that context. Also, spi.c does not consider the execCxt to be an exported part of its abstraction, and I'm pretty loath to punch another hole in that API. Also, as I've been working through this, I've found that only a rather small minority of plpgsql statements actually need statement-lifetime storage. So I'm thinking that it will be faster to create such a context only on-demand, not unconditionally; which knocks out any thought of changing plpgsql's coding conventions so much that statement-lifespan storage would become the normal place for CurrentMemoryContext to point. regards, tom lane -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers