On 29/01/11 21:27, Steve Singer wrote:
> On 11-01-27 04:33 PM, Jan Urbański wrote:
>>> I am finding the treatment of savepoints very strange.
>>> If as a function author I'm able to recover from errors then I'd expect
>>> (or maybe want) to be able to manage them through savepoints
>> Ooops, you found a bug there. In the attached patch you get the same
>> error (SPI_ERROR_TRANSACTION) as in master. Also added a unit test for
>> that.
>>
> I think you need to make the same change to PLy_spi_execute_plan.
D'oh, you're right, thanks. Attached patch with fix. Curiosly, right now
in master your example with plpy.prepare will result in "savepoint"
being swallowed, but it's of course better to react with an error.
Cheers,
Jan
diff --git a/src/pl/plpython/expected/plpython_error.out b/src/pl/plpython/expected/plpython_error.out
index ea4a54c..c8ce984 100644
*** a/src/pl/plpython/expected/plpython_error.out
--- b/src/pl/plpython/expected/plpython_error.out
*************** CREATE FUNCTION sql_syntax_error() RETUR
*** 8,15 ****
'plpy.execute("syntax error")'
LANGUAGE plpythonu;
SELECT sql_syntax_error();
- WARNING: plpy.SPIError: unrecognized error in PLy_spi_execute_query
- CONTEXT: PL/Python function "sql_syntax_error"
ERROR: plpy.SPIError: syntax error at or near "syntax"
LINE 1: syntax error
^
--- 8,13 ----
*************** CREATE FUNCTION exception_index_invalid_
*** 32,39 ****
return rv[0]'
LANGUAGE plpythonu;
SELECT exception_index_invalid_nested();
- WARNING: plpy.SPIError: unrecognized error in PLy_spi_execute_query
- CONTEXT: PL/Python function "exception_index_invalid_nested"
ERROR: plpy.SPIError: function test5(unknown) does not exist
LINE 1: SELECT test5('foo')
^
--- 30,35 ----
*************** return None
*** 54,61 ****
'
LANGUAGE plpythonu;
SELECT invalid_type_uncaught('rick');
- WARNING: plpy.SPIError: unrecognized error in PLy_spi_prepare
- CONTEXT: PL/Python function "invalid_type_uncaught"
ERROR: plpy.SPIError: type "test" does not exist
CONTEXT: PL/Python function "invalid_type_uncaught"
/* for what it's worth catch the exception generated by
--- 50,55 ----
*************** return None
*** 77,84 ****
'
LANGUAGE plpythonu;
SELECT invalid_type_caught('rick');
- WARNING: plpy.SPIError: unrecognized error in PLy_spi_prepare
- CONTEXT: PL/Python function "invalid_type_caught"
NOTICE: type "test" does not exist
CONTEXT: PL/Python function "invalid_type_caught"
invalid_type_caught
--- 71,76 ----
*************** return None
*** 104,111 ****
'
LANGUAGE plpythonu;
SELECT invalid_type_reraised('rick');
- WARNING: plpy.SPIError: unrecognized error in PLy_spi_prepare
- CONTEXT: PL/Python function "invalid_type_reraised"
ERROR: plpy.Error: type "test" does not exist
CONTEXT: PL/Python function "invalid_type_reraised"
/* no typo no messing about
--- 96,101 ----
*************** SELECT valid_type('rick');
*** 126,128 ****
--- 116,140 ----
(1 row)
+ /* manually starting subtransactions - a bad idea
+ */
+ CREATE FUNCTION manual_subxact() RETURNS void AS $$
+ plpy.execute("savepoint save")
+ plpy.execute("create table foo(x integer)")
+ plpy.execute("rollback to save")
+ $$ LANGUAGE plpythonu;
+ SELECT manual_subxact();
+ ERROR: plpy.SPIError: SPI_execute failed: SPI_ERROR_TRANSACTION
+ CONTEXT: PL/Python function "manual_subxact"
+ /* same for prepared plans
+ */
+ CREATE FUNCTION manual_subxact_prepared() RETURNS void AS $$
+ save = plpy.prepare("savepoint save")
+ rollback = plpy.prepare("rollback to save")
+ plpy.execute(save)
+ plpy.execute("create table foo(x integer)")
+ plpy.execute(rollback)
+ $$ LANGUAGE plpythonu;
+ SELECT manual_subxact_prepared();
+ ERROR: plpy.SPIError: SPI_execute_plan failed: SPI_ERROR_TRANSACTION
+ CONTEXT: PL/Python function "manual_subxact_prepared"
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index aafe556..6d3566a 100644
*** a/src/pl/plpython/plpython.c
--- b/src/pl/plpython/plpython.c
*************** typedef int Py_ssize_t;
*** 101,106 ****
--- 101,107 ----
#include "nodes/makefuncs.h"
#include "parser/parse_type.h"
#include "tcop/tcopprot.h"
+ #include "access/xact.h"
#include "utils/builtins.h"
#include "utils/hsearch.h"
#include "utils/lsyscache.h"
*************** PLy_spi_prepare(PyObject *self, PyObject
*** 2817,2822 ****
--- 2818,2824 ----
char *query;
void *tmpplan;
volatile MemoryContext oldcontext;
+ volatile ResourceOwner oldowner;
int nargs;
if (!PyArg_ParseTuple(args, "s|O", &query, &list))
*************** PLy_spi_prepare(PyObject *self, PyObject
*** 2840,2845 ****
--- 2842,2852 ----
plan->args = nargs ? PLy_malloc(sizeof(PLyTypeInfo) * nargs) : NULL;
oldcontext = CurrentMemoryContext;
+ oldowner = CurrentResourceOwner;
+
+ BeginInternalSubTransaction(NULL);
+ MemoryContextSwitchTo(oldcontext);
+
PG_TRY();
{
int i;
*************** PLy_spi_prepare(PyObject *self, PyObject
*** 2919,2938 ****
if (plan->plan == NULL)
elog(ERROR, "SPI_saveplan failed: %s",
SPI_result_code_string(SPI_result));
}
PG_CATCH();
{
ErrorData *edata;
MemoryContextSwitchTo(oldcontext);
edata = CopyErrorData();
FlushErrorState();
Py_DECREF(plan);
Py_XDECREF(optr);
! if (!PyErr_Occurred())
! PLy_exception_set(PLy_exc_spi_error,
! "unrecognized error in PLy_spi_prepare");
! PLy_elog(WARNING, NULL);
PLy_spi_exception_set(edata);
return NULL;
}
--- 2926,2967 ----
if (plan->plan == NULL)
elog(ERROR, "SPI_saveplan failed: %s",
SPI_result_code_string(SPI_result));
+
+ /* Commit the inner transaction, return to outer xact context */
+ ReleaseCurrentSubTransaction();
+ MemoryContextSwitchTo(oldcontext);
+ CurrentResourceOwner = oldowner;
+
+ /*
+ * AtEOSubXact_SPI() should not have popped any SPI context, but just
+ * in case it did, make sure we remain connected.
+ */
+ SPI_restore_connection();
}
PG_CATCH();
{
ErrorData *edata;
+ /* Save error info */
MemoryContextSwitchTo(oldcontext);
edata = CopyErrorData();
FlushErrorState();
Py_DECREF(plan);
Py_XDECREF(optr);
!
! /* Abort the inner transaction */
! RollbackAndReleaseCurrentSubTransaction();
! MemoryContextSwitchTo(oldcontext);
! CurrentResourceOwner = oldowner;
!
! /*
! * If AtEOSubXact_SPI() popped any SPI context of the subxact, it
! * will have left us in a disconnected state. We need this hack to
! * return to connected state.
! */
! SPI_restore_connection();
!
! /* Make Python raise the exception */
PLy_spi_exception_set(edata);
return NULL;
}
*************** PLy_spi_execute_plan(PyObject *ob, PyObj
*** 2974,2979 ****
--- 3003,3009 ----
rv;
PLyPlanObject *plan;
volatile MemoryContext oldcontext;
+ volatile ResourceOwner oldowner;
PyObject *ret;
if (list != NULL)
*************** PLy_spi_execute_plan(PyObject *ob, PyObj
*** 3009,3014 ****
--- 3039,3050 ----
}
oldcontext = CurrentMemoryContext;
+ oldowner = CurrentResourceOwner;
+
+ BeginInternalSubTransaction(NULL);
+ /* Want to run inside function's memory context */
+ MemoryContextSwitchTo(oldcontext);
+
PG_TRY();
{
char *nulls;
*************** PLy_spi_execute_plan(PyObject *ob, PyObj
*** 3061,3072 ****
--- 3097,3120 ----
if (nargs > 0)
pfree(nulls);
+
+ /* Commit the inner transaction, return to outer xact context */
+ ReleaseCurrentSubTransaction();
+ MemoryContextSwitchTo(oldcontext);
+ CurrentResourceOwner = oldowner;
+
+ /*
+ * AtEOSubXact_SPI() should not have popped any SPI context, but just
+ * in case it did, make sure we remain connected.
+ */
+ SPI_restore_connection();
}
PG_CATCH();
{
int k;
ErrorData *edata;
+ /* Save error info */
MemoryContextSwitchTo(oldcontext);
edata = CopyErrorData();
FlushErrorState();
*************** PLy_spi_execute_plan(PyObject *ob, PyObj
*** 3084,3093 ****
}
}
! if (!PyErr_Occurred())
! PLy_exception_set(PLy_exc_spi_error,
! "unrecognized error in PLy_spi_execute_plan");
! PLy_elog(WARNING, NULL);
PLy_spi_exception_set(edata);
return NULL;
}
--- 3132,3150 ----
}
}
! /* Abort the inner transaction */
! RollbackAndReleaseCurrentSubTransaction();
! MemoryContextSwitchTo(oldcontext);
! CurrentResourceOwner = oldowner;
!
! /*
! * If AtEOSubXact_SPI() popped any SPI context of the subxact, it
! * will have left us in a disconnected state. We need this hack to
! * return to connected state.
! */
! SPI_restore_connection();
!
! /* Make Python raise the exception */
PLy_spi_exception_set(edata);
return NULL;
}
*************** PLy_spi_execute_plan(PyObject *ob, PyObj
*** 3103,3108 ****
--- 3160,3173 ----
}
}
+ if (rv < 0)
+ {
+ PLy_exception_set(PLy_exc_spi_error,
+ "SPI_execute_plan failed: %s",
+ SPI_result_code_string(rv));
+ return NULL;
+ }
+
return ret;
}
*************** PLy_spi_execute_query(char *query, long
*** 3111,3136 ****
{
int rv;
volatile MemoryContext oldcontext;
PyObject *ret;
oldcontext = CurrentMemoryContext;
PG_TRY();
{
pg_verifymbstr(query, strlen(query), false);
rv = SPI_execute(query, PLy_curr_procedure->fn_readonly, limit);
ret = PLy_spi_execute_fetch_result(SPI_tuptable, SPI_processed, rv);
}
PG_CATCH();
{
ErrorData *edata;
MemoryContextSwitchTo(oldcontext);
edata = CopyErrorData();
FlushErrorState();
! if (!PyErr_Occurred())
! PLy_exception_set(PLy_exc_spi_error,
! "unrecognized error in PLy_spi_execute_query");
! PLy_elog(WARNING, NULL);
PLy_spi_exception_set(edata);
return NULL;
}
--- 3176,3230 ----
{
int rv;
volatile MemoryContext oldcontext;
+ volatile ResourceOwner oldowner;
PyObject *ret;
oldcontext = CurrentMemoryContext;
+ oldowner = CurrentResourceOwner;
+
+ BeginInternalSubTransaction(NULL);
+ /* Want to run inside function's memory context */
+ MemoryContextSwitchTo(oldcontext);
+
PG_TRY();
{
pg_verifymbstr(query, strlen(query), false);
rv = SPI_execute(query, PLy_curr_procedure->fn_readonly, limit);
ret = PLy_spi_execute_fetch_result(SPI_tuptable, SPI_processed, rv);
+
+ /* Commit the inner transaction, return to outer xact context */
+ ReleaseCurrentSubTransaction();
+ MemoryContextSwitchTo(oldcontext);
+ CurrentResourceOwner = oldowner;
+
+ /*
+ * AtEOSubXact_SPI() should not have popped any SPI context, but just
+ * in case it did, make sure we remain connected.
+ */
+ SPI_restore_connection();
}
PG_CATCH();
{
ErrorData *edata;
+ /* Save error info */
MemoryContextSwitchTo(oldcontext);
edata = CopyErrorData();
FlushErrorState();
!
! /* Abort the inner transaction */
! RollbackAndReleaseCurrentSubTransaction();
! MemoryContextSwitchTo(oldcontext);
! CurrentResourceOwner = oldowner;
!
! /*
! * If AtEOSubXact_SPI() popped any SPI context of the subxact, it
! * will have left us in a disconnected state. We need this hack to
! * return to connected state.
! */
! SPI_restore_connection();
!
! /* Make Python raise the exception */
PLy_spi_exception_set(edata);
return NULL;
}
diff --git a/src/pl/plpython/sql/plpython_error.sql b/src/pl/plpython/sql/plpython_error.sql
index 5ca6849..0571629 100644
*** a/src/pl/plpython/sql/plpython_error.sql
--- b/src/pl/plpython/sql/plpython_error.sql
*************** return None
*** 107,109 ****
--- 107,131 ----
LANGUAGE plpythonu;
SELECT valid_type('rick');
+
+ /* manually starting subtransactions - a bad idea
+ */
+ CREATE FUNCTION manual_subxact() RETURNS void AS $$
+ plpy.execute("savepoint save")
+ plpy.execute("create table foo(x integer)")
+ plpy.execute("rollback to save")
+ $$ LANGUAGE plpythonu;
+
+ SELECT manual_subxact();
+
+ /* same for prepared plans
+ */
+ CREATE FUNCTION manual_subxact_prepared() RETURNS void AS $$
+ save = plpy.prepare("savepoint save")
+ rollback = plpy.prepare("rollback to save")
+ plpy.execute(save)
+ plpy.execute("create table foo(x integer)")
+ plpy.execute(rollback)
+ $$ LANGUAGE plpythonu;
+
+ SELECT manual_subxact_prepared();
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
