From 2e3ea5017701cadd4ced2429f54c457afff12813 Mon Sep 17 00:00:00 2001 From: Honza Horak Date: Tue, 31 May 2022 22:08:28 +0200 Subject: [PATCH] Fix handling of errors during transaction with Python 3.11 Resolves: #2023272 --- ...ng-of-errors-during-transaction-comm.patch | 1096 +++++++++++++++++ postgresql.spec | 9 +- 2 files changed, 1104 insertions(+), 1 deletion(-) create mode 100644 postgresql-SPI-s-handling-of-errors-during-transaction-comm.patch diff --git a/postgresql-SPI-s-handling-of-errors-during-transaction-comm.patch b/postgresql-SPI-s-handling-of-errors-during-transaction-comm.patch new file mode 100644 index 0000000..6b7ecdd --- /dev/null +++ b/postgresql-SPI-s-handling-of-errors-during-transaction-comm.patch @@ -0,0 +1,1096 @@ +From 2e517818f4af4abe93bf56442469944544f10d4b Mon Sep 17 00:00:00 2001 +From: Tom Lane +Date: Mon, 28 Feb 2022 12:45:36 -0500 +Subject: [PATCH] Fix SPI's handling of errors during transaction commit. + +SPI_commit previously left it up to the caller to recover from any error +occurring during commit. Since that's complicated and requires use of +low-level xact.c facilities, it's not too surprising that no caller got +it right. Let's move the responsibility for cleanup into spi.c. Doing +that requires redefining SPI_commit as starting a new transaction, so +that it becomes equivalent to SPI_commit_and_chain except that you get +default transaction characteristics instead of preserving the prior +transaction's characteristics. We can make this pretty transparent +API-wise by redefining SPI_start_transaction() as a no-op. Callers +that expect to do something in between might be surprised, but +available evidence is that no callers do so. + +Having made that API redefinition, we can fix this mess by having +SPI_commit[_and_chain] trap errors and start a new, clean transaction +before re-throwing the error. Likewise for SPI_rollback[_and_chain]. +Some cleanup is also needed in AtEOXact_SPI, which was nowhere near +smart enough to deal with SPI contexts nested inside a committing +context. + +While plperl and pltcl need no changes beyond removing their now-useless +SPI_start_transaction() calls, plpython needs some more work because it +hadn't gotten the memo about catching commit/rollback errors in the +first place. Such an error resulted in longjmp'ing out of the Python +interpreter, which leaks Python stack entries at present and is reported +to crash Python 3.11 altogether. Add the missing logic to catch such +errors and convert them into Python exceptions. + +We are probably going to have to back-patch this once Python 3.11 ships, +but it's a sufficiently basic change that I'm a bit nervous about doing +so immediately. Let's let it bake awhile in HEAD first. + +Peter Eisentraut and Tom Lane + +Discussion: https://postgr.es/m/3375ffd8-d71c-2565-e348-a597d6e739e3@enterprisedb.com +Discussion: https://postgr.es/m/17416-ed8fe5d7213d6c25@postgresql.org +--- + doc/src/sgml/spi.sgml | 51 ++-- + src/backend/executor/spi.c | 221 +++++++++++++----- + src/backend/tcop/postgres.c | 2 - + src/backend/utils/mmgr/portalmem.c | 2 +- + src/include/executor/spi.h | 1 - + src/pl/plperl/expected/plperl_transaction.out | 48 ++++ + src/pl/plperl/plperl.c | 2 - + src/pl/plperl/sql/plperl_transaction.sql | 32 +++ + src/pl/plpgsql/src/pl_exec.c | 6 - + .../expected/plpython_transaction.out | 67 +++++- + src/pl/plpython/plpy_plpymodule.c | 30 --- + src/pl/plpython/plpy_spi.c | 94 ++++++++ + src/pl/plpython/plpy_spi.h | 3 + + src/pl/plpython/sql/plpython_transaction.sql | 30 +++ + src/pl/tcl/expected/pltcl_transaction.out | 49 ++++ + src/pl/tcl/pltcl.c | 2 - + src/pl/tcl/sql/pltcl_transaction.sql | 37 +++ + 17 files changed, 535 insertions(+), 142 deletions(-) + +diff --git a/doc/src/sgml/spi.sgml b/doc/src/sgml/spi.sgml +index d710e2d0df..7581661fc4 100644 +--- a/doc/src/sgml/spi.sgml ++++ b/doc/src/sgml/spi.sgml +@@ -99,10 +99,9 @@ int SPI_connect_ext(int options) + + + Sets the SPI connection to be nonatomic, which +- means that transaction control calls SPI_commit, +- SPI_rollback, and +- SPI_start_transaction are allowed. Otherwise, +- calling these functions will result in an immediate error. ++ means that transaction control calls (SPI_commit, ++ SPI_rollback) are allowed. Otherwise, ++ calling those functions will result in an immediate error. + + + +@@ -5040,15 +5039,17 @@ void SPI_commit_and_chain(void) + + SPI_commit commits the current transaction. It is + approximately equivalent to running the SQL +- command COMMIT. After a transaction is committed, a new +- transaction has to be started +- using SPI_start_transaction before further database +- actions can be executed. ++ command COMMIT. After the transaction is committed, a ++ new transaction is automatically started using default transaction ++ characteristics, so that the caller can continue using SPI facilities. ++ If there is a failure during commit, the current transaction is instead ++ rolled back and a new transaction is started, after which the error is ++ thrown in the usual way. + + + +- SPI_commit_and_chain is the same, but a new +- transaction is immediately started with the same transaction ++ SPI_commit_and_chain is the same, but the new ++ transaction is started with the same transaction + characteristics as the just finished one, like with the SQL command + COMMIT AND CHAIN. + +@@ -5093,14 +5094,13 @@ void SPI_rollback_and_chain(void) + + SPI_rollback rolls back the current transaction. It + is approximately equivalent to running the SQL +- command ROLLBACK. After a transaction is rolled back, a +- new transaction has to be started +- using SPI_start_transaction before further database +- actions can be executed. ++ command ROLLBACK. After the transaction is rolled back, ++ a new transaction is automatically started using default transaction ++ characteristics, so that the caller can continue using SPI facilities. + + +- SPI_rollback_and_chain is the same, but a new +- transaction is immediately started with the same transaction ++ SPI_rollback_and_chain is the same, but the new ++ transaction is started with the same transaction + characteristics as the just finished one, like with the SQL command + ROLLBACK AND CHAIN. + +@@ -5124,7 +5124,7 @@ void SPI_rollback_and_chain(void) + + + SPI_start_transaction +- start a new transaction ++ obsolete function + + + +@@ -5137,17 +5137,12 @@ void SPI_start_transaction(void) + Description + + +- SPI_start_transaction starts a new transaction. It +- can only be called after SPI_commit +- or SPI_rollback, as there is no transaction active at +- that point. Normally, when an SPI-using procedure is called, there is already a +- transaction active, so attempting to start another one before closing out +- the current one will result in an error. +- +- +- +- This function can only be executed if the SPI connection has been set as +- nonatomic in the call to SPI_connect_ext. ++ SPI_start_transaction does nothing, and exists ++ only for code compatibility with ++ earlier PostgreSQL releases. It used to ++ be required after calling SPI_commit ++ or SPI_rollback, but now those functions start ++ a new transaction automatically. + + + +diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c +index c93f90de9b..7971050746 100644 +--- a/src/backend/executor/spi.c ++++ b/src/backend/executor/spi.c +@@ -156,7 +156,8 @@ SPI_connect_ext(int options) + * XXX It could be better to use PortalContext as the parent context in + * all cases, but we may not be inside a portal (consider deferred-trigger + * execution). Perhaps CurTransactionContext could be an option? For now +- * it doesn't matter because we clean up explicitly in AtEOSubXact_SPI(). ++ * it doesn't matter because we clean up explicitly in AtEOSubXact_SPI(); ++ * but see also AtEOXact_SPI(). + */ + _SPI_current->procCxt = AllocSetContextCreate(_SPI_current->atomic ? TopTransactionContext : PortalContext, + "SPI Proc", +@@ -214,13 +215,13 @@ SPI_finish(void) + return SPI_OK_FINISH; + } + ++/* ++ * SPI_start_transaction is a no-op, kept for backwards compatibility. ++ * SPI callers are *always* inside a transaction. ++ */ + void + SPI_start_transaction(void) + { +- MemoryContext oldcontext = CurrentMemoryContext; +- +- StartTransactionCommand(); +- MemoryContextSwitchTo(oldcontext); + } + + static void +@@ -228,6 +229,12 @@ _SPI_commit(bool chain) + { + MemoryContext oldcontext = CurrentMemoryContext; + ++ /* ++ * Complain if we are in a context that doesn't permit transaction ++ * termination. (Note: here and _SPI_rollback should be the only places ++ * that throw ERRCODE_INVALID_TRANSACTION_TERMINATION, so that callers can ++ * test for that with security that they know what happened.) ++ */ + if (_SPI_current->atomic) + ereport(ERROR, + (errcode(ERRCODE_INVALID_TRANSACTION_TERMINATION), +@@ -240,40 +247,74 @@ _SPI_commit(bool chain) + * top-level transaction in such a block violates that idea. A future PL + * implementation might have different ideas about this, in which case + * this restriction would have to be refined or the check possibly be +- * moved out of SPI into the PLs. ++ * moved out of SPI into the PLs. Note however that the code below relies ++ * on not being within a subtransaction. + */ + if (IsSubTransaction()) + ereport(ERROR, + (errcode(ERRCODE_INVALID_TRANSACTION_TERMINATION), + errmsg("cannot commit while a subtransaction is active"))); + +- /* +- * Hold any pinned portals that any PLs might be using. We have to do +- * this before changing transaction state, since this will run +- * user-defined code that might throw an error. +- */ +- HoldPinnedPortals(); ++ /* XXX this ain't re-entrant enough for my taste */ ++ if (chain) ++ SaveTransactionCharacteristics(); + +- /* Start the actual commit */ +- _SPI_current->internal_xact = true; ++ /* Catch any error occurring during the COMMIT */ ++ PG_TRY(); ++ { ++ /* Protect current SPI stack entry against deletion */ ++ _SPI_current->internal_xact = true; + +- /* Release snapshots associated with portals */ +- ForgetPortalSnapshots(); ++ /* ++ * Hold any pinned portals that any PLs might be using. We have to do ++ * this before changing transaction state, since this will run ++ * user-defined code that might throw an error. ++ */ ++ HoldPinnedPortals(); + +- if (chain) +- SaveTransactionCharacteristics(); ++ /* Release snapshots associated with portals */ ++ ForgetPortalSnapshots(); + +- CommitTransactionCommand(); ++ /* Do the deed */ ++ CommitTransactionCommand(); + +- if (chain) +- { ++ /* Immediately start a new transaction */ + StartTransactionCommand(); +- RestoreTransactionCharacteristics(); ++ if (chain) ++ RestoreTransactionCharacteristics(); ++ ++ MemoryContextSwitchTo(oldcontext); ++ ++ _SPI_current->internal_xact = false; + } ++ PG_CATCH(); ++ { ++ ErrorData *edata; + +- MemoryContextSwitchTo(oldcontext); ++ /* Save error info in caller's context */ ++ MemoryContextSwitchTo(oldcontext); ++ edata = CopyErrorData(); ++ FlushErrorState(); + +- _SPI_current->internal_xact = false; ++ /* ++ * Abort the failed transaction. If this fails too, we'll just ++ * propagate the error out ... there's not that much we can do. ++ */ ++ AbortCurrentTransaction(); ++ ++ /* ... and start a new one */ ++ StartTransactionCommand(); ++ if (chain) ++ RestoreTransactionCharacteristics(); ++ ++ MemoryContextSwitchTo(oldcontext); ++ ++ _SPI_current->internal_xact = false; ++ ++ /* Now that we've cleaned up the transaction, re-throw the error */ ++ ReThrowError(edata); ++ } ++ PG_END_TRY(); + } + + void +@@ -293,6 +334,7 @@ _SPI_rollback(bool chain) + { + MemoryContext oldcontext = CurrentMemoryContext; + ++ /* see under SPI_commit() */ + if (_SPI_current->atomic) + ereport(ERROR, + (errcode(ERRCODE_INVALID_TRANSACTION_TERMINATION), +@@ -304,34 +346,68 @@ _SPI_rollback(bool chain) + (errcode(ERRCODE_INVALID_TRANSACTION_TERMINATION), + errmsg("cannot roll back while a subtransaction is active"))); + +- /* +- * Hold any pinned portals that any PLs might be using. We have to do +- * this before changing transaction state, since this will run +- * user-defined code that might throw an error, and in any case couldn't +- * be run in an already-aborted transaction. +- */ +- HoldPinnedPortals(); ++ /* XXX this ain't re-entrant enough for my taste */ ++ if (chain) ++ SaveTransactionCharacteristics(); + +- /* Start the actual rollback */ +- _SPI_current->internal_xact = true; ++ /* Catch any error occurring during the ROLLBACK */ ++ PG_TRY(); ++ { ++ /* Protect current SPI stack entry against deletion */ ++ _SPI_current->internal_xact = true; + +- /* Release snapshots associated with portals */ +- ForgetPortalSnapshots(); ++ /* ++ * Hold any pinned portals that any PLs might be using. We have to do ++ * this before changing transaction state, since this will run ++ * user-defined code that might throw an error, and in any case ++ * couldn't be run in an already-aborted transaction. ++ */ ++ HoldPinnedPortals(); + +- if (chain) +- SaveTransactionCharacteristics(); ++ /* Release snapshots associated with portals */ ++ ForgetPortalSnapshots(); + +- AbortCurrentTransaction(); ++ /* Do the deed */ ++ AbortCurrentTransaction(); + +- if (chain) +- { ++ /* Immediately start a new transaction */ + StartTransactionCommand(); +- RestoreTransactionCharacteristics(); ++ if (chain) ++ RestoreTransactionCharacteristics(); ++ ++ MemoryContextSwitchTo(oldcontext); ++ ++ _SPI_current->internal_xact = false; + } ++ PG_CATCH(); ++ { ++ ErrorData *edata; + +- MemoryContextSwitchTo(oldcontext); ++ /* Save error info in caller's context */ ++ MemoryContextSwitchTo(oldcontext); ++ edata = CopyErrorData(); ++ FlushErrorState(); + +- _SPI_current->internal_xact = false; ++ /* ++ * Try again to abort the failed transaction. If this fails too, ++ * we'll just propagate the error out ... there's not that much we can ++ * do. ++ */ ++ AbortCurrentTransaction(); ++ ++ /* ... and start a new one */ ++ StartTransactionCommand(); ++ if (chain) ++ RestoreTransactionCharacteristics(); ++ ++ MemoryContextSwitchTo(oldcontext); ++ ++ _SPI_current->internal_xact = false; ++ ++ /* Now that we've cleaned up the transaction, re-throw the error */ ++ ReThrowError(edata); ++ } ++ PG_END_TRY(); + } + + void +@@ -346,38 +422,55 @@ SPI_rollback_and_chain(void) + _SPI_rollback(true); + } + +-/* +- * Clean up SPI state. Called on transaction end (of non-SPI-internal +- * transactions) and when returning to the main loop on error. +- */ +-void +-SPICleanup(void) +-{ +- _SPI_current = NULL; +- _SPI_connected = -1; +- /* Reset API global variables, too */ +- SPI_processed = 0; +- SPI_tuptable = NULL; +- SPI_result = 0; +-} +- + /* + * Clean up SPI state at transaction commit or abort. + */ + void + AtEOXact_SPI(bool isCommit) + { +- /* Do nothing if the transaction end was initiated by SPI. */ +- if (_SPI_current && _SPI_current->internal_xact) +- return; ++ bool found = false; + +- if (isCommit && _SPI_connected != -1) ++ /* ++ * Pop stack entries, stopping if we find one marked internal_xact (that ++ * one belongs to the caller of SPI_commit or SPI_abort). ++ */ ++ while (_SPI_connected >= 0) ++ { ++ _SPI_connection *connection = &(_SPI_stack[_SPI_connected]); ++ ++ if (connection->internal_xact) ++ break; ++ ++ found = true; ++ ++ /* ++ * We need not release the procedure's memory contexts explicitly, as ++ * they'll go away automatically when their parent context does; see ++ * notes in SPI_connect_ext. ++ */ ++ ++ /* ++ * Restore outer global variables and pop the stack entry. Unlike ++ * SPI_finish(), we don't risk switching to memory contexts that might ++ * be already gone. ++ */ ++ SPI_processed = connection->outer_processed; ++ SPI_tuptable = connection->outer_tuptable; ++ SPI_result = connection->outer_result; ++ ++ _SPI_connected--; ++ if (_SPI_connected < 0) ++ _SPI_current = NULL; ++ else ++ _SPI_current = &(_SPI_stack[_SPI_connected]); ++ } ++ ++ /* We should only find entries to pop during an ABORT. */ ++ if (found && isCommit) + ereport(WARNING, + (errcode(ERRCODE_WARNING), + errmsg("transaction left non-empty SPI stack"), + errhint("Check for missing \"SPI_finish\" calls."))); +- +- SPICleanup(); + } + + /* +diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c +index 3c7d08209f..34c13a1113 100644 +--- a/src/backend/tcop/postgres.c ++++ b/src/backend/tcop/postgres.c +@@ -42,7 +42,6 @@ + #include "catalog/pg_type.h" + #include "commands/async.h" + #include "commands/prepare.h" +-#include "executor/spi.h" + #include "jit/jit.h" + #include "libpq/libpq.h" + #include "libpq/pqformat.h" +@@ -4253,7 +4252,6 @@ PostgresMain(int argc, char *argv[], + WalSndErrorCleanup(); + + PortalErrorCleanup(); +- SPICleanup(); + + /* + * We can't release replication slots inside AbortTransaction() as we +diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c +index 21ad87c024..afc03682d9 100644 +--- a/src/backend/utils/mmgr/portalmem.c ++++ b/src/backend/utils/mmgr/portalmem.c +@@ -1261,7 +1261,7 @@ HoldPinnedPortals(void) + */ + if (portal->strategy != PORTAL_ONE_SELECT) + ereport(ERROR, +- (errcode(ERRCODE_INVALID_TRANSACTION_TERMINATION), ++ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot perform transaction commands inside a cursor loop that is not read-only"))); + + /* Verify it's in a suitable state to be held */ +diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h +index e20e7df780..6ec3851444 100644 +--- a/src/include/executor/spi.h ++++ b/src/include/executor/spi.h +@@ -205,7 +205,6 @@ extern void SPI_commit_and_chain(void); + extern void SPI_rollback(void); + extern void SPI_rollback_and_chain(void); + +-extern void SPICleanup(void); + extern void AtEOXact_SPI(bool isCommit); + extern void AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid); + extern bool SPI_inside_nonatomic_context(void); +diff --git a/src/pl/plperl/expected/plperl_transaction.out b/src/pl/plperl/expected/plperl_transaction.out +index 7ca0ef35fb..da4283cbce 100644 +--- a/src/pl/plperl/expected/plperl_transaction.out ++++ b/src/pl/plperl/expected/plperl_transaction.out +@@ -192,5 +192,53 @@ SELECT * FROM pg_cursors; + ------+-----------+-------------+-----------+---------------+--------------- + (0 rows) + ++-- check handling of an error during COMMIT ++CREATE TABLE testpk (id int PRIMARY KEY); ++CREATE TABLE testfk(f1 int REFERENCES testpk DEFERRABLE INITIALLY DEFERRED); ++DO LANGUAGE plperl $$ ++# this insert will fail during commit: ++spi_exec_query("INSERT INTO testfk VALUES (0)"); ++spi_commit(); ++elog(WARNING, 'should not get here'); ++$$; ++ERROR: insert or update on table "testfk" violates foreign key constraint "testfk_f1_fkey" at line 4. ++CONTEXT: PL/Perl anonymous code block ++SELECT * FROM testpk; ++ id ++---- ++(0 rows) ++ ++SELECT * FROM testfk; ++ f1 ++---- ++(0 rows) ++ ++DO LANGUAGE plperl $$ ++# this insert will fail during commit: ++spi_exec_query("INSERT INTO testfk VALUES (0)"); ++eval { ++ spi_commit(); ++}; ++if ($@) { ++ elog(INFO, $@); ++} ++# these inserts should work: ++spi_exec_query("INSERT INTO testpk VALUES (1)"); ++spi_exec_query("INSERT INTO testfk VALUES (1)"); ++$$; ++INFO: insert or update on table "testfk" violates foreign key constraint "testfk_f1_fkey" at line 5. ++ ++SELECT * FROM testpk; ++ id ++---- ++ 1 ++(1 row) ++ ++SELECT * FROM testfk; ++ f1 ++---- ++ 1 ++(1 row) ++ + DROP TABLE test1; + DROP TABLE test2; +diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c +index 81bb480bc2..edb93ec1c4 100644 +--- a/src/pl/plperl/plperl.c ++++ b/src/pl/plperl/plperl.c +@@ -3986,7 +3986,6 @@ plperl_spi_commit(void) + PG_TRY(); + { + SPI_commit(); +- SPI_start_transaction(); + } + PG_CATCH(); + { +@@ -4013,7 +4012,6 @@ plperl_spi_rollback(void) + PG_TRY(); + { + SPI_rollback(); +- SPI_start_transaction(); + } + PG_CATCH(); + { +diff --git a/src/pl/plperl/sql/plperl_transaction.sql b/src/pl/plperl/sql/plperl_transaction.sql +index 0a60799805..d10c8bee89 100644 +--- a/src/pl/plperl/sql/plperl_transaction.sql ++++ b/src/pl/plperl/sql/plperl_transaction.sql +@@ -159,5 +159,37 @@ SELECT * FROM test1; + SELECT * FROM pg_cursors; + + ++-- check handling of an error during COMMIT ++CREATE TABLE testpk (id int PRIMARY KEY); ++CREATE TABLE testfk(f1 int REFERENCES testpk DEFERRABLE INITIALLY DEFERRED); ++ ++DO LANGUAGE plperl $$ ++# this insert will fail during commit: ++spi_exec_query("INSERT INTO testfk VALUES (0)"); ++spi_commit(); ++elog(WARNING, 'should not get here'); ++$$; ++ ++SELECT * FROM testpk; ++SELECT * FROM testfk; ++ ++DO LANGUAGE plperl $$ ++# this insert will fail during commit: ++spi_exec_query("INSERT INTO testfk VALUES (0)"); ++eval { ++ spi_commit(); ++}; ++if ($@) { ++ elog(INFO, $@); ++} ++# these inserts should work: ++spi_exec_query("INSERT INTO testpk VALUES (1)"); ++spi_exec_query("INSERT INTO testfk VALUES (1)"); ++$$; ++ ++SELECT * FROM testpk; ++SELECT * FROM testfk; ++ ++ + DROP TABLE test1; + DROP TABLE test2; +diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c +index 9674c29250..915139378e 100644 +--- a/src/pl/plpgsql/src/pl_exec.c ++++ b/src/pl/plpgsql/src/pl_exec.c +@@ -4916,10 +4916,7 @@ exec_stmt_commit(PLpgSQL_execstate *estate, PLpgSQL_stmt_commit *stmt) + if (stmt->chain) + SPI_commit_and_chain(); + else +- { + SPI_commit(); +- SPI_start_transaction(); +- } + + /* + * We need to build new simple-expression infrastructure, since the old +@@ -4943,10 +4940,7 @@ exec_stmt_rollback(PLpgSQL_execstate *estate, PLpgSQL_stmt_rollback *stmt) + if (stmt->chain) + SPI_rollback_and_chain(); + else +- { + SPI_rollback(); +- SPI_start_transaction(); +- } + + /* + * We need to build new simple-expression infrastructure, since the old +diff --git a/src/pl/plpython/expected/plpython_transaction.out b/src/pl/plpython/expected/plpython_transaction.out +index 14152993c7..72d1e45a76 100644 +--- a/src/pl/plpython/expected/plpython_transaction.out ++++ b/src/pl/plpython/expected/plpython_transaction.out +@@ -55,8 +55,11 @@ for i in range(0, 10): + return 1 + $$; + SELECT transaction_test2(); +-ERROR: invalid transaction termination +-CONTEXT: PL/Python function "transaction_test2" ++ERROR: spiexceptions.InvalidTransactionTermination: invalid transaction termination ++CONTEXT: Traceback (most recent call last): ++ PL/Python function "transaction_test2", line 5, in ++ plpy.commit() ++PL/Python function "transaction_test2" + SELECT * FROM test1; + a | b + ---+--- +@@ -70,7 +73,7 @@ plpy.execute("CALL transaction_test1()") + return 1 + $$; + SELECT transaction_test3(); +-ERROR: spiexceptions.InvalidTransactionTermination: invalid transaction termination ++ERROR: spiexceptions.InvalidTransactionTermination: spiexceptions.InvalidTransactionTermination: invalid transaction termination + CONTEXT: Traceback (most recent call last): + PL/Python function "transaction_test3", line 2, in + plpy.execute("CALL transaction_test1()") +@@ -88,7 +91,7 @@ plpy.execute("DO LANGUAGE plpythonu $x$ plpy.commit() $x$") + return 1 + $$; + SELECT transaction_test4(); +-ERROR: spiexceptions.InvalidTransactionTermination: invalid transaction termination ++ERROR: spiexceptions.InvalidTransactionTermination: spiexceptions.InvalidTransactionTermination: invalid transaction termination + CONTEXT: Traceback (most recent call last): + PL/Python function "transaction_test4", line 2, in + plpy.execute("DO LANGUAGE plpythonu $x$ plpy.commit() $x$") +@@ -100,8 +103,11 @@ s.enter() + plpy.commit() + $$; + WARNING: forcibly aborting a subtransaction that has not been exited +-ERROR: cannot commit while a subtransaction is active +-CONTEXT: PL/Python anonymous code block ++ERROR: spiexceptions.InvalidTransactionTermination: cannot commit while a subtransaction is active ++CONTEXT: Traceback (most recent call last): ++ PL/Python anonymous code block, line 4, in ++ plpy.commit() ++PL/Python anonymous code block + -- commit inside cursor loop + CREATE TABLE test2 (x int); + INSERT INTO test2 VALUES (0), (1), (2), (3), (4); +@@ -191,5 +197,54 @@ SELECT * FROM pg_cursors; + ------+-----------+-------------+-----------+---------------+--------------- + (0 rows) + ++-- check handling of an error during COMMIT ++CREATE TABLE testpk (id int PRIMARY KEY); ++CREATE TABLE testfk(f1 int REFERENCES testpk DEFERRABLE INITIALLY DEFERRED); ++DO LANGUAGE plpythonu $$ ++# this insert will fail during commit: ++plpy.execute("INSERT INTO testfk VALUES (0)") ++plpy.commit() ++plpy.warning('should not get here') ++$$; ++ERROR: spiexceptions.ForeignKeyViolation: insert or update on table "testfk" violates foreign key constraint "testfk_f1_fkey" ++DETAIL: Key (f1)=(0) is not present in table "testpk". ++CONTEXT: Traceback (most recent call last): ++ PL/Python anonymous code block, line 4, in ++ plpy.commit() ++PL/Python anonymous code block ++SELECT * FROM testpk; ++ id ++---- ++(0 rows) ++ ++SELECT * FROM testfk; ++ f1 ++---- ++(0 rows) ++ ++DO LANGUAGE plpythonu $$ ++# this insert will fail during commit: ++plpy.execute("INSERT INTO testfk VALUES (0)") ++try: ++ plpy.commit() ++except Exception as e: ++ plpy.info('sqlstate: %s' % (e.sqlstate)) ++# these inserts should work: ++plpy.execute("INSERT INTO testpk VALUES (1)") ++plpy.execute("INSERT INTO testfk VALUES (1)") ++$$; ++INFO: sqlstate: 23503 ++SELECT * FROM testpk; ++ id ++---- ++ 1 ++(1 row) ++ ++SELECT * FROM testfk; ++ f1 ++---- ++ 1 ++(1 row) ++ + DROP TABLE test1; + DROP TABLE test2; +diff --git a/src/pl/plpython/plpy_plpymodule.c b/src/pl/plpython/plpy_plpymodule.c +index 0365acc95b..907f89d153 100644 +--- a/src/pl/plpython/plpy_plpymodule.c ++++ b/src/pl/plpython/plpy_plpymodule.c +@@ -40,8 +40,6 @@ static PyObject *PLy_fatal(PyObject *self, PyObject *args, PyObject *kw); + static PyObject *PLy_quote_literal(PyObject *self, PyObject *args); + static PyObject *PLy_quote_nullable(PyObject *self, PyObject *args); + static PyObject *PLy_quote_ident(PyObject *self, PyObject *args); +-static PyObject *PLy_commit(PyObject *self, PyObject *args); +-static PyObject *PLy_rollback(PyObject *self, PyObject *args); + + + /* A list of all known exceptions, generated from backend/utils/errcodes.txt */ +@@ -577,31 +575,3 @@ PLy_output(volatile int level, PyObject *self, PyObject *args, PyObject *kw) + */ + Py_RETURN_NONE; + } +- +-static PyObject * +-PLy_commit(PyObject *self, PyObject *args) +-{ +- PLyExecutionContext *exec_ctx = PLy_current_execution_context(); +- +- SPI_commit(); +- SPI_start_transaction(); +- +- /* was cleared at transaction end, reset pointer */ +- exec_ctx->scratch_ctx = NULL; +- +- Py_RETURN_NONE; +-} +- +-static PyObject * +-PLy_rollback(PyObject *self, PyObject *args) +-{ +- PLyExecutionContext *exec_ctx = PLy_current_execution_context(); +- +- SPI_rollback(); +- SPI_start_transaction(); +- +- /* was cleared at transaction end, reset pointer */ +- exec_ctx->scratch_ctx = NULL; +- +- Py_RETURN_NONE; +-} +diff --git a/src/pl/plpython/plpy_spi.c b/src/pl/plpython/plpy_spi.c +index 99c1b4f28f..86d70470a7 100644 +--- a/src/pl/plpython/plpy_spi.c ++++ b/src/pl/plpython/plpy_spi.c +@@ -456,6 +456,100 @@ PLy_spi_execute_fetch_result(SPITupleTable *tuptable, uint64 rows, int status) + return (PyObject *) result; + } + ++PyObject * ++PLy_commit(PyObject *self, PyObject *args) ++{ ++ MemoryContext oldcontext = CurrentMemoryContext; ++ PLyExecutionContext *exec_ctx = PLy_current_execution_context(); ++ ++ PG_TRY(); ++ { ++ SPI_commit(); ++ ++ /* was cleared at transaction end, reset pointer */ ++ exec_ctx->scratch_ctx = NULL; ++ } ++ PG_CATCH(); ++ { ++ ErrorData *edata; ++ PLyExceptionEntry *entry; ++ PyObject *exc; ++ ++ /* Save error info */ ++ MemoryContextSwitchTo(oldcontext); ++ edata = CopyErrorData(); ++ FlushErrorState(); ++ ++ /* was cleared at transaction end, reset pointer */ ++ exec_ctx->scratch_ctx = NULL; ++ ++ /* Look up the correct exception */ ++ entry = hash_search(PLy_spi_exceptions, &(edata->sqlerrcode), ++ HASH_FIND, NULL); ++ ++ /* ++ * This could be a custom error code, if that's the case fallback to ++ * SPIError ++ */ ++ exc = entry ? entry->exc : PLy_exc_spi_error; ++ /* Make Python raise the exception */ ++ PLy_spi_exception_set(exc, edata); ++ FreeErrorData(edata); ++ ++ return NULL; ++ } ++ PG_END_TRY(); ++ ++ Py_RETURN_NONE; ++} ++ ++PyObject * ++PLy_rollback(PyObject *self, PyObject *args) ++{ ++ MemoryContext oldcontext = CurrentMemoryContext; ++ PLyExecutionContext *exec_ctx = PLy_current_execution_context(); ++ ++ PG_TRY(); ++ { ++ SPI_rollback(); ++ ++ /* was cleared at transaction end, reset pointer */ ++ exec_ctx->scratch_ctx = NULL; ++ } ++ PG_CATCH(); ++ { ++ ErrorData *edata; ++ PLyExceptionEntry *entry; ++ PyObject *exc; ++ ++ /* Save error info */ ++ MemoryContextSwitchTo(oldcontext); ++ edata = CopyErrorData(); ++ FlushErrorState(); ++ ++ /* was cleared at transaction end, reset pointer */ ++ exec_ctx->scratch_ctx = NULL; ++ ++ /* Look up the correct exception */ ++ entry = hash_search(PLy_spi_exceptions, &(edata->sqlerrcode), ++ HASH_FIND, NULL); ++ ++ /* ++ * This could be a custom error code, if that's the case fallback to ++ * SPIError ++ */ ++ exc = entry ? entry->exc : PLy_exc_spi_error; ++ /* Make Python raise the exception */ ++ PLy_spi_exception_set(exc, edata); ++ FreeErrorData(edata); ++ ++ return NULL; ++ } ++ PG_END_TRY(); ++ ++ Py_RETURN_NONE; ++} ++ + /* + * Utilities for running SPI functions in subtransactions. + * +diff --git a/src/pl/plpython/plpy_spi.h b/src/pl/plpython/plpy_spi.h +index a5e2e60da7..98ccd21093 100644 +--- a/src/pl/plpython/plpy_spi.h ++++ b/src/pl/plpython/plpy_spi.h +@@ -12,6 +12,9 @@ extern PyObject *PLy_spi_prepare(PyObject *self, PyObject *args); + extern PyObject *PLy_spi_execute(PyObject *self, PyObject *args); + extern PyObject *PLy_spi_execute_plan(PyObject *ob, PyObject *list, long limit); + ++extern PyObject *PLy_commit(PyObject *self, PyObject *args); ++extern PyObject *PLy_rollback(PyObject *self, PyObject *args); ++ + typedef struct PLyExceptionEntry + { + int sqlstate; /* hash key, must be first */ +diff --git a/src/pl/plpython/sql/plpython_transaction.sql b/src/pl/plpython/sql/plpython_transaction.sql +index 33b37e5b7f..68588d9fb0 100644 +--- a/src/pl/plpython/sql/plpython_transaction.sql ++++ b/src/pl/plpython/sql/plpython_transaction.sql +@@ -148,5 +148,35 @@ SELECT * FROM test1; + SELECT * FROM pg_cursors; + + ++-- check handling of an error during COMMIT ++CREATE TABLE testpk (id int PRIMARY KEY); ++CREATE TABLE testfk(f1 int REFERENCES testpk DEFERRABLE INITIALLY DEFERRED); ++ ++DO LANGUAGE plpythonu $$ ++# this insert will fail during commit: ++plpy.execute("INSERT INTO testfk VALUES (0)") ++plpy.commit() ++plpy.warning('should not get here') ++$$; ++ ++SELECT * FROM testpk; ++SELECT * FROM testfk; ++ ++DO LANGUAGE plpythonu $$ ++# this insert will fail during commit: ++plpy.execute("INSERT INTO testfk VALUES (0)") ++try: ++ plpy.commit() ++except Exception as e: ++ plpy.info('sqlstate: %s' % (e.sqlstate)) ++# these inserts should work: ++plpy.execute("INSERT INTO testpk VALUES (1)") ++plpy.execute("INSERT INTO testfk VALUES (1)") ++$$; ++ ++SELECT * FROM testpk; ++SELECT * FROM testfk; ++ ++ + DROP TABLE test1; + DROP TABLE test2; +diff --git a/src/pl/tcl/expected/pltcl_transaction.out b/src/pl/tcl/expected/pltcl_transaction.out +index 007204b99a..f557b79138 100644 +--- a/src/pl/tcl/expected/pltcl_transaction.out ++++ b/src/pl/tcl/expected/pltcl_transaction.out +@@ -96,5 +96,54 @@ SELECT * FROM test1; + ---+--- + (0 rows) + ++-- check handling of an error during COMMIT ++CREATE TABLE testpk (id int PRIMARY KEY); ++CREATE TABLE testfk(f1 int REFERENCES testpk DEFERRABLE INITIALLY DEFERRED); ++CREATE PROCEDURE transaction_testfk() ++LANGUAGE pltcl ++AS $$ ++# this insert will fail during commit: ++spi_exec "INSERT INTO testfk VALUES (0)" ++commit ++elog WARNING "should not get here" ++$$; ++CALL transaction_testfk(); ++ERROR: insert or update on table "testfk" violates foreign key constraint "testfk_f1_fkey" ++SELECT * FROM testpk; ++ id ++---- ++(0 rows) ++ ++SELECT * FROM testfk; ++ f1 ++---- ++(0 rows) ++ ++CREATE OR REPLACE PROCEDURE transaction_testfk() ++LANGUAGE pltcl ++AS $$ ++# this insert will fail during commit: ++spi_exec "INSERT INTO testfk VALUES (0)" ++if [catch {commit} msg] { ++ elog INFO $msg ++} ++# these inserts should work: ++spi_exec "INSERT INTO testpk VALUES (1)" ++spi_exec "INSERT INTO testfk VALUES (1)" ++$$; ++CALL transaction_testfk(); ++INFO: insert or update on table "testfk" violates foreign key constraint "testfk_f1_fkey" ++SELECT * FROM testpk; ++ id ++---- ++ 1 ++(1 row) ++ ++SELECT * FROM testfk; ++ f1 ++---- ++ 1 ++(1 row) ++ + DROP TABLE test1; + DROP TABLE test2; +diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c +index c5fad05e12..68c9bd1970 100644 +--- a/src/pl/tcl/pltcl.c ++++ b/src/pl/tcl/pltcl.c +@@ -2935,7 +2935,6 @@ pltcl_commit(ClientData cdata, Tcl_Interp *interp, + PG_TRY(); + { + SPI_commit(); +- SPI_start_transaction(); + } + PG_CATCH(); + { +@@ -2975,7 +2974,6 @@ pltcl_rollback(ClientData cdata, Tcl_Interp *interp, + PG_TRY(); + { + SPI_rollback(); +- SPI_start_transaction(); + } + PG_CATCH(); + { +diff --git a/src/pl/tcl/sql/pltcl_transaction.sql b/src/pl/tcl/sql/pltcl_transaction.sql +index c752faf665..bd759850a7 100644 +--- a/src/pl/tcl/sql/pltcl_transaction.sql ++++ b/src/pl/tcl/sql/pltcl_transaction.sql +@@ -94,5 +94,42 @@ CALL transaction_test4b(); + SELECT * FROM test1; + + ++-- check handling of an error during COMMIT ++CREATE TABLE testpk (id int PRIMARY KEY); ++CREATE TABLE testfk(f1 int REFERENCES testpk DEFERRABLE INITIALLY DEFERRED); ++ ++CREATE PROCEDURE transaction_testfk() ++LANGUAGE pltcl ++AS $$ ++# this insert will fail during commit: ++spi_exec "INSERT INTO testfk VALUES (0)" ++commit ++elog WARNING "should not get here" ++$$; ++ ++CALL transaction_testfk(); ++ ++SELECT * FROM testpk; ++SELECT * FROM testfk; ++ ++CREATE OR REPLACE PROCEDURE transaction_testfk() ++LANGUAGE pltcl ++AS $$ ++# this insert will fail during commit: ++spi_exec "INSERT INTO testfk VALUES (0)" ++if [catch {commit} msg] { ++ elog INFO $msg ++} ++# these inserts should work: ++spi_exec "INSERT INTO testpk VALUES (1)" ++spi_exec "INSERT INTO testfk VALUES (1)" ++$$; ++ ++CALL transaction_testfk(); ++ ++SELECT * FROM testpk; ++SELECT * FROM testfk; ++ ++ + DROP TABLE test1; + DROP TABLE test2; +-- +2.35.1 + diff --git a/postgresql.spec b/postgresql.spec index 2c5c54e..e6413d1 100644 --- a/postgresql.spec +++ b/postgresql.spec @@ -65,7 +65,7 @@ Summary: PostgreSQL client programs Name: postgresql %global majorversion 14 Version: %{majorversion}.3 -Release: 2%{?dist} +Release: 3%{?dist} # The PostgreSQL license is very similar to other MIT licenses, but the OSI # recognizes it as an independent license, so we do as well. @@ -118,6 +118,8 @@ Patch10: postgresql-datalayout-mismatch-on-s390.patch Patch12: postgresql-no-libecpg.patch # This patch disables deprecated ciphers in the test suite Patch14: postgresql-pgcrypto-openssl3-tests.patch +# Fix compatibility with Python 3.11 +Patch15: postgresql-SPI-s-handling-of-errors-during-transaction-comm.patch BuildRequires: make BuildRequires: gcc @@ -435,6 +437,7 @@ goal of accelerating analytics queries. %patch9 -p1 %patch10 -p1 %patch14 -p1 +%patch15 -p1 # We used to run autoconf here, but there's no longer any real need to, # since Postgres ships with a reasonably modern configure script. @@ -1253,6 +1256,10 @@ make -C postgresql-setup-%{setup_version} check %changelog +* Mon Jun 06 2022 Honza Horak - 14.3-3 +- Fix handling of errors during transaction with Python 3.11 + Resolves: #2023272 + * Wed Jun 01 2022 Jitka Plesnikova - 14.3-2 - Perl 5.36 rebuild