Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nw7nJ-0006b2-By for pgsql-bugs@arkaria.postgresql.org; Tue, 31 May 2022 19:44:45 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1nw7nH-0005fr-Ue for pgsql-bugs@arkaria.postgresql.org; Tue, 31 May 2022 19:44:43 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nw7nH-0005fd-Dl for pgsql-bugs@lists.postgresql.org; Tue, 31 May 2022 19:44:43 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nw7nD-0002GC-HY for pgsql-bugs@lists.postgresql.org; Tue, 31 May 2022 19:44:42 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 24VJicUk316827; Tue, 31 May 2022 15:44:38 -0400 From: Tom Lane To: Honza Horak cc: pgsql-bugs@lists.postgresql.org Subject: Re: PostgreSQL 14.3 plpython not compatible with Python 3.11 In-reply-to: References: Comments: In-reply-to Honza Horak message dated "Tue, 31 May 2022 20:47:18 +0200" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <316665.1654026260.0@sss.pgh.pa.us> Date: Tue, 31 May 2022 15:44:38 -0400 Message-ID: <316826.1654026278@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <316665.1654026260.1@sss.pgh.pa.us> Honza Horak writes: > Now I read more carefully that this issue was already mentioned by Tom in > the original bug: > https://bugzilla.redhat.com/show_bug.cgi?id=2023272#c7 > I was just confused not seeing the fix in the latest version... will try it. Here's a quick rebase of that commit onto the v14 branch, in case it helps. regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="fix-spi-commit-error-handling-v14.patch"; charset="us-ascii" Content-ID: <316665.1654026260.2@sss.pgh.pa.us> Content-Description: fix-spi-commit-error-handling-v14.patch Content-Transfer-Encoding: quoted-printable 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, wh= ich - 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 i= s 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 databa= se - 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 instea= d + 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 ba= ck, a - new transaction has to be started - using SPI_start_transaction before further databa= se - 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 activ= e 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 f4d090dbd7..dd5ef76270 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-trigg= er * execution). Perhaps CurTransactionContext could be an option? For n= ow - * 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 =3D AllocSetContextCreate(_SPI_current->atomic ? T= opTransactionContext : 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 =3D CurrentMemoryContext; - - StartTransactionCommand(); - MemoryContextSwitchTo(oldcontext); } = static void @@ -228,6 +229,12 @@ _SPI_commit(bool chain) { MemoryContext oldcontext =3D 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 c= an + * 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 P= L * 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 reli= es + * 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 =3D true; + /* Catch any error occurring during the COMMIT */ + PG_TRY(); + { + /* Protect current SPI stack entry against deletion */ + _SPI_current->internal_xact =3D 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 =3D false; } + PG_CATCH(); + { + ErrorData *edata; = - MemoryContextSwitchTo(oldcontext); + /* Save error info in caller's context */ + MemoryContextSwitchTo(oldcontext); + edata =3D CopyErrorData(); + FlushErrorState(); = - _SPI_current->internal_xact =3D 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 =3D 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 =3D 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 =3D true; + /* Catch any error occurring during the ROLLBACK */ + PG_TRY(); + { + /* Protect current SPI stack entry against deletion */ + _SPI_current->internal_xact =3D 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 =3D false; } + PG_CATCH(); + { + ErrorData *edata; = - MemoryContextSwitchTo(oldcontext); + /* Save error info in caller's context */ + MemoryContextSwitchTo(oldcontext); + edata =3D CopyErrorData(); + FlushErrorState(); = - _SPI_current->internal_xact =3D 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 =3D 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 =3D NULL; - _SPI_connected =3D -1; - /* Reset API global variables, too */ - SPI_processed =3D 0; - SPI_tuptable =3D NULL; - SPI_result =3D 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 =3D false; = - if (isCommit && _SPI_connected !=3D -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 >=3D 0) + { + _SPI_connection *connection =3D &(_SPI_stack[_SPI_connected]); + + if (connection->internal_xact) + break; + + found =3D 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 =3D connection->outer_processed; + SPI_tuptable =3D connection->outer_tuptable; + SPI_result =3D connection->outer_result; + + _SPI_connected--; + if (_SPI_connected < 0) + _SPI_current =3D NULL; + else + _SPI_current =3D &(_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 2c50575b37..fe81c55698 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/p= ortalmem.c index 58674d611d..d93c59100c 100644 --- a/src/backend/utils/mmgr/portalmem.c +++ b/src/backend/utils/mmgr/portalmem.c @@ -1275,7 +1275,7 @@ HoldPinnedPortals(void) */ if (portal->strategy !=3D 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 t= hat 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 1e66a7d2ea..153eb5c7ad 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 DEFERRE= D); +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 constrain= t "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 a346c18e8e..48591e4842 100644 --- a/src/pl/plperl/plperl.c +++ b/src/pl/plperl/plperl.c @@ -3983,7 +3983,6 @@ plperl_spi_commit(void) PG_TRY(); { SPI_commit(); - SPI_start_transaction(); } PG_CATCH(); { @@ -4010,7 +4009,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 DEFERRE= D); + +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 796adf960c..6bd8ee4fc0 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -4890,10 +4890,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 @@ -4917,10 +4914,7 @@ exec_stmt_rollback(PLpgSQL_execstate *estate, PLpgS= QL_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/pl= python/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.Invali= dTransactionTermination: 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.Invali= dTransactionTermination: 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 DEFERRE= D); +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 "tes= tfk" violates foreign key constraint "testfk_f1_fkey" +DETAIL: Key (f1)=3D(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_plpy= module.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 *arg= s, 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, PyObje= ct *args, PyObject *kw) */ Py_RETURN_NONE; } - -static PyObject * -PLy_commit(PyObject *self, PyObject *args) -{ - PLyExecutionContext *exec_ctx =3D PLy_current_execution_context(); - - SPI_commit(); - SPI_start_transaction(); - - /* was cleared at transaction end, reset pointer */ - exec_ctx->scratch_ctx =3D NULL; - - Py_RETURN_NONE; -} - -static PyObject * -PLy_rollback(PyObject *self, PyObject *args) -{ - PLyExecutionContext *exec_ctx =3D PLy_current_execution_context(); - - SPI_rollback(); - SPI_start_transaction(); - - /* was cleared at transaction end, reset pointer */ - exec_ctx->scratch_ctx =3D 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 =3D CurrentMemoryContext; + PLyExecutionContext *exec_ctx =3D PLy_current_execution_context(); + + PG_TRY(); + { + SPI_commit(); + + /* was cleared at transaction end, reset pointer */ + exec_ctx->scratch_ctx =3D NULL; + } + PG_CATCH(); + { + ErrorData *edata; + PLyExceptionEntry *entry; + PyObject *exc; + + /* Save error info */ + MemoryContextSwitchTo(oldcontext); + edata =3D CopyErrorData(); + FlushErrorState(); + + /* was cleared at transaction end, reset pointer */ + exec_ctx->scratch_ctx =3D NULL; + + /* Look up the correct exception */ + entry =3D 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 =3D 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 =3D CurrentMemoryContext; + PLyExecutionContext *exec_ctx =3D PLy_current_execution_context(); + + PG_TRY(); + { + SPI_rollback(); + + /* was cleared at transaction end, reset pointer */ + exec_ctx->scratch_ctx =3D NULL; + } + PG_CATCH(); + { + ErrorData *edata; + PLyExceptionEntry *entry; + PyObject *exc; + + /* Save error info */ + MemoryContextSwitchTo(oldcontext); + edata =3D CopyErrorData(); + FlushErrorState(); + + /* was cleared at transaction end, reset pointer */ + exec_ctx->scratch_ctx =3D NULL; + + /* Look up the correct exception */ + entry =3D 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 =3D 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, PyObjec= t *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/plpytho= n/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 DEFERRE= D); + +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/expect= ed/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 DEFERRE= D); +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 constrain= t "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 e11837559d..4ce61a585d 100644 --- a/src/pl/tcl/pltcl.c +++ b/src/pl/tcl/pltcl.c @@ -2928,7 +2928,6 @@ pltcl_commit(ClientData cdata, Tcl_Interp *interp, PG_TRY(); { SPI_commit(); - SPI_start_transaction(); } PG_CATCH(); { @@ -2968,7 +2967,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_t= ransaction.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 DEFERRE= D); + +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; ------- =_aaaaaaaaaa0--