]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Fix SPI's handling of errors during transaction commit.
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 22 Jun 2022 16:12:00 +0000 (12:12 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 22 Jun 2022 16:12:00 +0000 (12:12 -0400)
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.

This is a back-patch of commit 2e517818f.  That's now aged long enough
to reduce the concerns about whether it will break something, and we
do need to ensure that supported branches will work with Python 3.11.

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

17 files changed:
doc/src/sgml/spi.sgml
src/backend/executor/spi.c
src/backend/tcop/postgres.c
src/backend/utils/mmgr/portalmem.c
src/include/executor/spi.h
src/pl/plperl/expected/plperl_transaction.out
src/pl/plperl/plperl.c
src/pl/plperl/sql/plperl_transaction.sql
src/pl/plpgsql/src/pl_exec.c
src/pl/plpython/expected/plpython_transaction.out
src/pl/plpython/plpy_plpymodule.c
src/pl/plpython/plpy_spi.c
src/pl/plpython/plpy_spi.h
src/pl/plpython/sql/plpython_transaction.sql
src/pl/tcl/expected/pltcl_transaction.out
src/pl/tcl/pltcl.c
src/pl/tcl/sql/pltcl_transaction.sql

index 22e6bd32629a1247d1665ffcf2858df23b35211a..dea10e98eaf52eff6678f75e2f333acc32f011b4 100644 (file)
@@ -99,10 +99,9 @@ int SPI_connect_ext(int <parameter>options</parameter>)
      <listitem>
       <para>
        Sets the SPI connection to be <firstterm>nonatomic</firstterm>, which
-       means that transaction control calls <function>SPI_commit</function>,
-       <function>SPI_rollback</function>, and
-       <function>SPI_start_transaction</function> are allowed.  Otherwise,
-       calling these functions will result in an immediate error.
+       means that transaction control calls (<function>SPI_commit</function>,
+       <function>SPI_rollback</function>) are allowed.  Otherwise,
+       calling those functions will result in an immediate error.
       </para>
      </listitem>
     </varlistentry>
@@ -4399,10 +4398,12 @@ void SPI_commit(void)
   <para>
    <function>SPI_commit</function> commits the current transaction.  It is
    approximately equivalent to running the SQL
-   command <command>COMMIT</command>.  After a transaction is committed, a new
-   transaction has to be started
-   using <function>SPI_start_transaction</function> before further database
-   actions can be executed.
+   command <command>COMMIT</command>.  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.
   </para>
 
   <para>
@@ -4439,10 +4440,9 @@ void SPI_rollback(void)
   <para>
    <function>SPI_rollback</function> rolls back the current transaction.  It
    is approximately equivalent to running the SQL
-   command <command>ROLLBACK</command>.  After a transaction is rolled back, a
-   new transaction has to be started
-   using <function>SPI_start_transaction</function> before further database
-   actions can be executed.
+   command <command>ROLLBACK</command>.  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.
   </para>
 
   <para>
@@ -4464,7 +4464,7 @@ void SPI_rollback(void)
 
  <refnamediv>
   <refname>SPI_start_transaction</refname>
-  <refpurpose>start a new transaction</refpurpose>
+  <refpurpose>obsolete function</refpurpose>
  </refnamediv>
 
  <refsynopsisdiv>
@@ -4477,17 +4477,12 @@ void SPI_start_transaction(void)
   <title>Description</title>
 
   <para>
-   <function>SPI_start_transaction</function> starts a new transaction.  It
-   can only be called after <function>SPI_commit</function>
-   or <function>SPI_rollback</function>, 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.
-  </para>
-
-  <para>
-   This function can only be executed if the SPI connection has been set as
-   nonatomic in the call to <function>SPI_connect_ext</function>.
+   <function>SPI_start_transaction</function> does nothing, and exists
+   only for code compatibility with
+   earlier <productname>PostgreSQL</productname> releases.  It used to
+   be required after calling <function>SPI_commit</function>
+   or <function>SPI_rollback</function>, but now those functions start
+   a new transaction automatically.
   </para>
  </refsect1>
 </refentry>
index 59b826c835edea6110eb0f6479ef9d07120af4f1..0f5932c93bc191537b6d07db264c93591358d427 100644 (file)
@@ -153,7 +153,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",
@@ -213,13 +214,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);
 }
 
 void
@@ -227,6 +228,12 @@ SPI_commit(void)
 {
        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),
@@ -239,30 +246,66 @@ SPI_commit(void)
         * 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();
+       /* Catch any error occurring during the COMMIT */
+       PG_TRY();
+       {
+               /* Protect current SPI stack entry against deletion */
+               _SPI_current->internal_xact = true;
 
-       /* Start the actual commit */
-       _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();
+               /* Release snapshots associated with portals */
+               ForgetPortalSnapshots();
 
-       CommitTransactionCommand();
-       MemoryContextSwitchTo(oldcontext);
+               /* Do the deed */
+               CommitTransactionCommand();
 
-       _SPI_current->internal_xact = false;
+               /* Immediately start a new transaction */
+               StartTransactionCommand();
+
+               MemoryContextSwitchTo(oldcontext);
+
+               _SPI_current->internal_xact = false;
+       }
+       PG_CATCH();
+       {
+               ErrorData  *edata;
+
+               /* Save error info in caller's context */
+               MemoryContextSwitchTo(oldcontext);
+               edata = CopyErrorData();
+               FlushErrorState();
+
+               /*
+                * 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();
+
+               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
@@ -270,6 +313,7 @@ SPI_rollback(void)
 {
        MemoryContext oldcontext = CurrentMemoryContext;
 
+       /* see under SPI_commit() */
        if (_SPI_current->atomic)
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_TRANSACTION_TERMINATION),
@@ -281,40 +325,60 @@ SPI_rollback(void)
                                (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();
+       /* Catch any error occurring during the ROLLBACK */
+       PG_TRY();
+       {
+               /* Protect current SPI stack entry against deletion */
+               _SPI_current->internal_xact = true;
 
-       /* Start the actual rollback */
-       _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();
+               /* Release snapshots associated with portals */
+               ForgetPortalSnapshots();
 
-       AbortCurrentTransaction();
-       MemoryContextSwitchTo(oldcontext);
+               /* Do the deed */
+               AbortCurrentTransaction();
 
-       _SPI_current->internal_xact = false;
-}
+               /* Immediately start a new transaction */
+               StartTransactionCommand();
 
-/*
- * 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_lastoid = InvalidOid;
-       SPI_tuptable = NULL;
-       SPI_result = 0;
+               MemoryContextSwitchTo(oldcontext);
+
+               _SPI_current->internal_xact = false;
+       }
+       PG_CATCH();
+       {
+               ErrorData  *edata;
+
+               /* Save error info in caller's context */
+               MemoryContextSwitchTo(oldcontext);
+               edata = CopyErrorData();
+               FlushErrorState();
+
+               /*
+                * 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();
+
+               MemoryContextSwitchTo(oldcontext);
+
+               _SPI_current->internal_xact = false;
+
+               /* Now that we've cleaned up the transaction, re-throw the error */
+               ReThrowError(edata);
+       }
+       PG_END_TRY();
 }
 
 /*
@@ -323,17 +387,49 @@ SPICleanup(void)
 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;
+
+       /*
+        * 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;
 
-       if (isCommit && _SPI_connected != -1)
+               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();
 }
 
 /*
index e033847538b818d4d67be8cdc90694b8e416c187..6f8e186b15d8dca19342c38fe82079a1f77e0cae 100644 (file)
@@ -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"
@@ -3993,7 +3992,6 @@ PostgresMain(int argc, char *argv[],
                        WalSndErrorCleanup();
 
                PortalErrorCleanup();
-               SPICleanup();
 
                /*
                 * We can't release replication slots inside AbortTransaction() as we
index e7a45423532a07a7422fadf5cb16e69667db6f2b..6dc8651d5b57b157ccca44929311c9d1f04f19e8 100644 (file)
@@ -1274,7 +1274,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 */
index b16440cf004bdbc8e166de0c1bdf03e21b3d2bfa..41cca05b274704fa6c08a8323f7200cbedafd0a1 100644 (file)
@@ -163,7 +163,6 @@ extern void SPI_start_transaction(void);
 extern void SPI_commit(void);
 extern void SPI_rollback(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);
index 7ca0ef35fb83c66f3cd0b3be4e08affcfdc5c766..da4283cbcedfa18f07f5bca70433c1c51beec41b 100644 (file)
@@ -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;
index f9c8f3d4013a6b0741ad0c559750ccdbe3e39408..e6b0f16908a46d2d189f83635e2db9d0eb7c0c9e 100644 (file)
@@ -3991,7 +3991,6 @@ plperl_spi_commit(void)
        PG_TRY();
        {
                SPI_commit();
-               SPI_start_transaction();
        }
        PG_CATCH();
        {
@@ -4018,7 +4017,6 @@ plperl_spi_rollback(void)
        PG_TRY();
        {
                SPI_rollback();
-               SPI_start_transaction();
        }
        PG_CATCH();
        {
index 0a60799805582eaaac67b2914e23cb3b8d54e4c6..d10c8bee8978be00457fdac125ddc4b7162da4c5 100644 (file)
@@ -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;
index a0669006cda49b6264fee55588ef530498c00932..d1e45c6cac6ccba9cf33b2244800068b4e9fb5f0 100644 (file)
@@ -4853,7 +4853,6 @@ static int
 exec_stmt_commit(PLpgSQL_execstate *estate, PLpgSQL_stmt_commit *stmt)
 {
        SPI_commit();
-       SPI_start_transaction();
 
        /*
         * We need to build new simple-expression infrastructure, since the old
@@ -4874,7 +4873,6 @@ static int
 exec_stmt_rollback(PLpgSQL_execstate *estate, PLpgSQL_stmt_rollback *stmt)
 {
        SPI_rollback();
-       SPI_start_transaction();
 
        /*
         * We need to build new simple-expression infrastructure, since the old
index 14152993c7516ba4628ffda1da2eb06447e3dcbf..72d1e45a768b9c2f7471b0e9f52fa814be6eb8bd 100644 (file)
@@ -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 <module>
+    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 <module>
     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 <module>
     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 <module>
+    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 <module>
+    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;
index 3d8983b3b3e48a70a7b36a06f81e5134c3dd995d..6ffd42a2c90b0e2519890497215bed77c63cda0f 100644 (file)
@@ -44,8 +44,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 */
@@ -588,31 +586,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;
-}
index 41155fc81ec7a10390716197de1ba0fe39b954c0..ab1f6ba9a59a56637fdb49107c6cdf5170e1a5cd 100644 (file)
@@ -461,6 +461,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.
  *
index 5a0eef78dcd9a5c4c9249b5879a994cd1b6f6847..d4351f8fb4ae3dca712510637a7da0decf81936d 100644 (file)
@@ -11,6 +11,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 */
index 33b37e5b7fcae80ffec64aa0a1da9523eeeb11ef..68588d9fb0282ef74d9be14168f06249db0954ae 100644 (file)
@@ -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;
index 007204b99adc1d1b5d6e6744587064e678f1785d..f557b791386db7da73c30857b1f162bf09d0a98a 100644 (file)
@@ -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;
index e2fa43b890de8aa4b80537835e2da021dc8881f2..577fda7f26e6fcb219ac6b0d57811d22bdcbc8a2 100644 (file)
@@ -2958,7 +2958,6 @@ pltcl_commit(ClientData cdata, Tcl_Interp *interp,
        PG_TRY();
        {
                SPI_commit();
-               SPI_start_transaction();
        }
        PG_CATCH();
        {
@@ -2998,7 +2997,6 @@ pltcl_rollback(ClientData cdata, Tcl_Interp *interp,
        PG_TRY();
        {
                SPI_rollback();
-               SPI_start_transaction();
        }
        PG_CATCH();
        {
index c752faf665422e099116f8e4c68da5ea91cd10f3..bd759850a7054fbf1787bae781f0ad0c335b1077 100644 (file)
@@ -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;