Re: fix crash with Python 3.11

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
Cc: exclusion(at)gmail(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: fix crash with Python 3.11
Date: 2022-02-23 19:31:22
Message-ID: 1216692.1645644682@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> We could provide a simplified API in which SPI_start_transaction is
> automatically included for either success or failure, so that the caller
> doesn't have the delayed-cleanup responsibility. I'm not actually sure
> that there is any situation where that couldn't be done, but putting it
> into the existing functions would be a API break (... unless we turn
> SPI_start_transaction into a no-op, which might be OK?) Basically this'd
> be making the behavior of SPI_commit_and_chain the norm, except you'd
> have the option of reverting to default transaction settings instead
> of the previous ones.
> So basically where we'd end up is that plpython would do about what
> you show in your patch, but then there's additional work needed in
> spi.c so that we're not leaving the rest of the system in a bad state.

Here's a draft patch that works that way. I haven't tested it against
Python 3.11, but it handles the case I showed upthread (now incorporated
as a regression test), as well as Alexander's repeat-till-stack-overflow
problem. The one thing I found that I wasn't expecting is that
AtEOXact_SPI is quite a few bricks shy of a load: it doesn't handle
cases where some atomic contexts are atop an internal_xact one. That
happens in the new test case because the error is thrown from RI
triggers that themselves use SPI.

This is draft mainly in that

* I didn't touch spi.sgml yet.

* It might be a good idea to add parallel test cases for the other PLs.

* I'm not satisfied with using static storage for
SaveTransactionCharacteristics/RestoreTransactionCharacteristics.
It'd be better for them to use a struct allocated locally in the caller.
That would be a simple enough change, but also an API break; on the
other hand, I really doubt anything outside core code is calling those.
Anyone object to changing them?

I'm not sure how to proceed with this. It's kind of a large change
to be putting into back branches, but our hands will be forced once
Python 3.11 is out. Maybe put it in HEAD now, and plan to back-patch
in a few months?

regards, tom lane

Attachment Content-Type Size
start-new-xact-in-SPI-commit-rollback-1.patch text/x-diff 20.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Matthias van de Meent 2022-02-23 19:50:59 Re: Report checkpoint progress with pg_stat_progress_checkpoint (was: Report checkpoint progress in server logs)
Previous Message Justin Pryzby 2022-02-23 19:22:10 Re: Report checkpoint progress with pg_stat_progress_checkpoint (was: Report checkpoint progress in server logs)