On 25.01.22 16:54, Tom Lane wrote:
> Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
>> On 16.01.22 23:53, Tom Lane wrote:
>>> I think a possible fix is:
>>>
>>> 1. Before entering the PG_TRY block, check for active subtransaction(s)
>>> and immediately throw a Python error if there is one. (This corresponds
>>> to the existing errors "cannot commit while a subtransaction is active"
>>> and "cannot roll back while a subtransaction is active". The point is
>>> to reduce the number of system states we have to worry about below.)
>
>> AFAICT, AbortOutOfAnyTransaction() also aborts subtransactions, so why
>> do you suggest the separate handling of subtransactions?
>
> We don't want these operations to be able to cancel subtransactions,
> do we? The existing errors certainly suggest not.
I've been struggling to make progress on this. First, throwing the
Python exception suggested in #1 above would require a significant
amount of new code. (We have code to create an exception out of
ErrorData, but no code to make one up from nothing.) This would need
further thought on how to arrange the code sensibly. Second, calling
AbortOutOfAnyTransaction() appears to clean up so much stuff that even
the data needed for error handling in PL/Python is wiped out. The
symptoms are various crashes on pointers now valued 0x7f7f7f... You fix
one, you get the next one. So more analysis would be required there, too.
One way to way to address the first problem is to not handle the "cannot
commit while a subtransaction is active" and similar cases manually but
let _SPI_commit() throw the error and then check in PL/Python for the
error code ERRCODE_INVALID_TRANSACTION_TERMINATION. (The code in
_SPI_commit() is there after all explicitly for PLs, so if we're not
using it, then we're doing it wrong.) And then only call
AbortOutOfAnyTransaction() (or whatever) if it's a different code, which
would mean something in the actual committing went wrong.
But in any case, for either implementation it seems then you'd get
different error behavior from .commit and .rollback calls depending on
the specific error, which seems weird.
Altogether, maybe the response to
> The existing test cases apparently fail to trip over that
> because Python just throws the exception again, but what if someone
> writes a plpython function that catches the exception and then tries
> to perform new SPI actions?
perhaps should be, don't do that then?
