diff --git a/doc/src/sgml/spi.sgml b/doc/src/sgml/spi.sgml index d710e2d0df3..7581661fc4a 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 c93f90de9b7..7971050746f 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(); - - /* Start the actual commit */ - _SPI_current->internal_xact = true; - - /* Release snapshots associated with portals */ - ForgetPortalSnapshots(); - + /* XXX this ain't re-entrant enough for my taste */ if (chain) SaveTransactionCharacteristics(); - CommitTransactionCommand(); - - if (chain) + /* Catch any error occurring during the COMMIT */ + PG_TRY(); { + /* Protect current SPI stack entry against deletion */ + _SPI_current->internal_xact = true; + + /* + * 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(); + + /* Release snapshots associated with portals */ + ForgetPortalSnapshots(); + + /* Do the deed */ + CommitTransactionCommand(); + + /* 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(); - - /* Start the actual rollback */ - _SPI_current->internal_xact = true; - - /* Release snapshots associated with portals */ - ForgetPortalSnapshots(); - + /* XXX this ain't re-entrant enough for my taste */ if (chain) SaveTransactionCharacteristics(); - AbortCurrentTransaction(); - - if (chain) + /* Catch any error occurring during the ROLLBACK */ + PG_TRY(); { + /* Protect current SPI stack entry against deletion */ + _SPI_current->internal_xact = true; + + /* + * 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(); + + /* Release snapshots associated with portals */ + ForgetPortalSnapshots(); + + /* Do the deed */ + AbortCurrentTransaction(); + + /* 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 3c7d08209f3..34c13a11138 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -43,7 +43,6 @@ #include "commands/async.h" #include "commands/prepare.h" #include "common/pg_prng.h" -#include "executor/spi.h" #include "jit/jit.h" #include "libpq/libpq.h" #include "libpq/pqformat.h" @@ -4263,7 +4262,6 @@ PostgresMain(const char *dbname, const char *username) 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 21ad87c0241..afc03682d9c 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 e20e7df780d..6ec38514445 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 7ca0ef35fb8..da4283cbced 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 81bb480bc20..edb93ec1c4c 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 0a607998055..d10c8bee897 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 9674c292505..915139378e6 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 14152993c75..72d1e45a768 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 0365acc95b0..907f89d1535 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 99c1b4f28f6..86d70470a74 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 a5e2e60da7f..98ccd210931 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 33b37e5b7fc..68588d9fb02 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 007204b99ad..f557b791386 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 c5fad05e122..68c9bd1970e 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 c752faf6654..bd759850a70 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;