Re: [PATCH] plpython function causes server panic

On Fri, Mar 22, 2024 at 4:37 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Fair enough. In the attached v2, I wrote "change the transaction
> state (other than by using a subtransaction for error recovery)";
> what do you think of that?

I think that's pretty good. I wonder if there are some bizarre cases
where the patch would allow slightly more than that ... who is to say
that you must pop the subtransaction you pushed? But that sort of
pedantry is probably not worth worrying about for purposes of the
documentation, especially because such a thing might not be a very
good idea anyway.

> I dug around in the docs and couldn't really find anything about
> parallel-query transaction limitations other than this bit in
> parallel.sgml and the more or less copy-pasted text in
> create_function.sgml; did you have any other spots in mind?
> (I did find the commentary in README.parallel, but that's not
> exactly user-facing.)

I don't have anything else in mind at the moment.

> I think these things are already dealt with. However, one thing
> worth questioning is that CommitSubTransaction() will just silently
> kill any workers started during the current subxact, and likewise
> CommitTransaction() zaps workers without complaint. Shouldn't these
> instead throw an error about how you didn't close parallel mode,
> and then the corresponding Abort function does the cleanup?
> I did not change that behavior here, but it seems dubious.

I'm not sure. I definitely knew when I wrote this code that we often
emit warnings about resources that aren't cleaned up at (sub)commit
time rather than just silently releasing them, and I feel like the
fact that I didn't implement that behavior here was probably a
deliberate choice to avoid some problem. But I have no memory of what
that problem was, and it is entirely possible that it was eliminated
at some later phase of development. I think that decision was made
quite early, before much of anything was working.

--
Robert Haas
EDB: http://www.enterprisedb.com