On 20/01/11 01:26, Jan Urbański wrote:
> On 19/01/11 10:57, Jan Urbański wrote:
>> On 18/01/11 23:22, Peter Eisentraut wrote:
>>> #2: It looks like this loses some information/formatting in the error
>>> message. Should we keep the pointing arrow there?
>
>>> CONTEXT: PL/Python function "sql_syntax_error"
>>> -ERROR: syntax error at or near "syntax"
>>> -LINE 1: syntax error
>>> - ^
>>> -QUERY: syntax error
>>> +ERROR: PL/Python: plpy.SPIError: syntax error at or near "syntax"
>>> CONTEXT: PL/Python function "sql_syntax_error"
>
>> Yes, the message is less informative, because the error is reported by
>> PL/Python, by wrapping the SPI message. I guess I could try to extract
>> more info from the caught ErrorData and put it in the new error that
>> PL/Python throws.
>
> All right, I found a way to shoehorn the extra information into
> SPIException, I'll post a new patch series with what's left of the
> general refactoring patch soon.
Here's an updated patch series for PL/Python refactoring. It was 16
patches at first, 8 are committed, 1 got dropped, so we're down to 7.
Cheers,
Jan
>From af0a83a4ff0356fe5b172b6c60692953d7e01bf0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Urba=C5=84ski?= <wulc...@wulczer.org>
Date: Sun, 2 Jan 2011 11:42:54 +0100
Subject: [PATCH 7/7] Do not prefix error messages with the string "PL/Python: ".
It is redundant, given the error context. Also, call pg_verifymbstr
outside of the try/catch block, because it can lead to errorneously
reporting a Python error when it is in fact a Postgres error.
---
src/pl/plpython/expected/plpython_do.out | 2 +-
src/pl/plpython/expected/plpython_error.out | 20 ++++++++++----------
src/pl/plpython/expected/plpython_test.out | 2 +-
src/pl/plpython/expected/plpython_types.out | 2 +-
src/pl/plpython/expected/plpython_types_3.out | 2 +-
src/pl/plpython/plpython.c | 13 ++++++-------
6 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/src/pl/plpython/expected/plpython_do.out b/src/pl/plpython/expected/plpython_do.out
index 9de261a..a21b088 100644
--- a/src/pl/plpython/expected/plpython_do.out
+++ b/src/pl/plpython/expected/plpython_do.out
@@ -2,5 +2,5 @@ DO $$ plpy.notice("This is plpythonu.") $$ LANGUAGE plpythonu;
NOTICE: This is plpythonu.
CONTEXT: PL/Python anonymous code block
DO $$ nonsense $$ LANGUAGE plpythonu;
-ERROR: PL/Python: NameError: global name 'nonsense' is not defined
+ERROR: NameError: global name 'nonsense' is not defined
CONTEXT: PL/Python anonymous code block
diff --git a/src/pl/plpython/expected/plpython_error.out b/src/pl/plpython/expected/plpython_error.out
index 87225f2..ea4a54c 100644
--- a/src/pl/plpython/expected/plpython_error.out
+++ b/src/pl/plpython/expected/plpython_error.out
@@ -8,9 +8,9 @@ CREATE FUNCTION sql_syntax_error() RETURNS text
'plpy.execute("syntax error")'
LANGUAGE plpythonu;
SELECT sql_syntax_error();
-WARNING: PL/Python: plpy.SPIError: unrecognized error in PLy_spi_execute_query
+WARNING: plpy.SPIError: unrecognized error in PLy_spi_execute_query
CONTEXT: PL/Python function "sql_syntax_error"
-ERROR: PL/Python: plpy.SPIError: syntax error at or near "syntax"
+ERROR: plpy.SPIError: syntax error at or near "syntax"
LINE 1: syntax error
^
QUERY: syntax error
@@ -22,7 +22,7 @@ CREATE FUNCTION exception_index_invalid(text) RETURNS text
'return args[1]'
LANGUAGE plpythonu;
SELECT exception_index_invalid('test');
-ERROR: PL/Python: IndexError: list index out of range
+ERROR: IndexError: list index out of range
CONTEXT: PL/Python function "exception_index_invalid"
/* check handling of nested exceptions
*/
@@ -32,9 +32,9 @@ CREATE FUNCTION exception_index_invalid_nested() RETURNS text
return rv[0]'
LANGUAGE plpythonu;
SELECT exception_index_invalid_nested();
-WARNING: PL/Python: plpy.SPIError: unrecognized error in PLy_spi_execute_query
+WARNING: plpy.SPIError: unrecognized error in PLy_spi_execute_query
CONTEXT: PL/Python function "exception_index_invalid_nested"
-ERROR: PL/Python: plpy.SPIError: function test5(unknown) does not exist
+ERROR: plpy.SPIError: function test5(unknown) does not exist
LINE 1: SELECT test5('foo')
^
HINT: No function matches the given name and argument types. You might need to add explicit type casts.
@@ -54,9 +54,9 @@ return None
'
LANGUAGE plpythonu;
SELECT invalid_type_uncaught('rick');
-WARNING: PL/Python: plpy.SPIError: unrecognized error in PLy_spi_prepare
+WARNING: plpy.SPIError: unrecognized error in PLy_spi_prepare
CONTEXT: PL/Python function "invalid_type_uncaught"
-ERROR: PL/Python: plpy.SPIError: type "test" does not exist
+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
* the typo, and return None
@@ -77,7 +77,7 @@ return None
'
LANGUAGE plpythonu;
SELECT invalid_type_caught('rick');
-WARNING: PL/Python: plpy.SPIError: unrecognized error in PLy_spi_prepare
+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"
@@ -104,9 +104,9 @@ return None
'
LANGUAGE plpythonu;
SELECT invalid_type_reraised('rick');
-WARNING: PL/Python: plpy.SPIError: unrecognized error in PLy_spi_prepare
+WARNING: plpy.SPIError: unrecognized error in PLy_spi_prepare
CONTEXT: PL/Python function "invalid_type_reraised"
-ERROR: PL/Python: plpy.Error: type "test" does not exist
+ERROR: plpy.Error: type "test" does not exist
CONTEXT: PL/Python function "invalid_type_reraised"
/* no typo no messing about
*/
diff --git a/src/pl/plpython/expected/plpython_test.out b/src/pl/plpython/expected/plpython_test.out
index 583a149..d92c987 100644
--- a/src/pl/plpython/expected/plpython_test.out
+++ b/src/pl/plpython/expected/plpython_test.out
@@ -73,5 +73,5 @@ NOTICE: notice
CONTEXT: PL/Python function "elog_test"
WARNING: warning
CONTEXT: PL/Python function "elog_test"
-ERROR: PL/Python: plpy.Error: error
+ERROR: plpy.Error: error
CONTEXT: PL/Python function "elog_test"
diff --git a/src/pl/plpython/expected/plpython_types.out b/src/pl/plpython/expected/plpython_types.out
index a165936..982005b 100644
--- a/src/pl/plpython/expected/plpython_types.out
+++ b/src/pl/plpython/expected/plpython_types.out
@@ -596,7 +596,7 @@ CREATE FUNCTION test_type_conversion_array_error() RETURNS int[] AS $$
return 5
$$ LANGUAGE plpythonu;
SELECT * FROM test_type_conversion_array_error();
-ERROR: PL/Python: return value of function with array return type is not a Python sequence
+ERROR: return value of function with array return type is not a Python sequence
CONTEXT: while creating return value
PL/Python function "test_type_conversion_array_error"
--
diff --git a/src/pl/plpython/expected/plpython_types_3.out b/src/pl/plpython/expected/plpython_types_3.out
index 38ddf02..eeb23b7 100644
--- a/src/pl/plpython/expected/plpython_types_3.out
+++ b/src/pl/plpython/expected/plpython_types_3.out
@@ -596,7 +596,7 @@ CREATE FUNCTION test_type_conversion_array_error() RETURNS int[] AS $$
return 5
$$ LANGUAGE plpython3u;
SELECT * FROM test_type_conversion_array_error();
-ERROR: PL/Python: return value of function with array return type is not a Python sequence
+ERROR: return value of function with array return type is not a Python sequence
CONTEXT: while creating return value
PL/Python function "test_type_conversion_array_error"
--
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index c07913c..9ad89eb 100644
--- a/src/pl/plpython/plpython.c
+++ b/src/pl/plpython/plpython.c
@@ -3460,9 +3460,9 @@ PLy_fatal(PyObject *self, PyObject *args)
static PyObject *
PLy_output(volatile int level, PyObject *self, PyObject *args)
{
- PyObject *volatile so;
- char *volatile sv;
- volatile MemoryContext oldcontext;
+ PyObject *volatile so;
+ char *volatile sv;
+ MemoryContext volatile oldcontext = CurrentMemoryContext;
if (PyTuple_Size(args) == 1)
{
@@ -3483,10 +3483,9 @@ PLy_output(volatile int level, PyObject *self, PyObject *args)
sv = dgettext(TEXTDOMAIN, "could not parse error message in plpy.elog");
}
- oldcontext = CurrentMemoryContext;
+ pg_verifymbstr(sv, strlen(sv), false);
PG_TRY();
{
- pg_verifymbstr(sv, strlen(sv), false);
elog(level, "%s", sv);
}
PG_CATCH();
@@ -3659,14 +3658,14 @@ PLy_elog(int elevel, const char *fmt,...)
{
if (fmt)
ereport(elevel,
- (errmsg("PL/Python: %s", emsg.data),
+ (errmsg("%s", emsg.data),
(xmsg) ? errdetail("%s", xmsg) : 0,
(hint) ? errhint(hint) : 0,
(query) ? internalerrquery(query) : 0,
(position) ? internalerrposition(position) : 0));
else
ereport(elevel,
- (errmsg("PL/Python: %s", xmsg),
+ (errmsg("%s", xmsg),
(hint) ? errhint(hint) : 0,
(query) ? internalerrquery(query) : 0,
(position) ? internalerrposition(position) : 0));
--
1.7.2.3
>From fe3246f4a967f5b305eed639cc2f3c33aac599ac Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Urba=C5=84ski?= <wulc...@wulczer.org>
Date: Sun, 2 Jan 2011 11:39:26 +0100
Subject: [PATCH 6/7] Improve exception usage in PL/Python.
Specifically, use the builtin ValueError, not SPIError for errors
having to do with argument counts or types. Use SPIError, not simply
plpy.Error for errors in PLy_spi_execute_plan. Finally, do not set a
Python exception if PyArg_ParseTuple failed, as it arleady sets the
correct exception.
---
src/pl/plpython/plpython.c | 14 ++++++--------
1 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index 1eeabb0..c07913c 100644
--- a/src/pl/plpython/plpython.c
+++ b/src/pl/plpython/plpython.c
@@ -2844,14 +2844,12 @@ PLy_spi_prepare(PyObject *self, PyObject *args)
if (!PyArg_ParseTuple(args, "s|O", &query, &list))
{
- PLy_exception_set(PLy_exc_spi_error,
- "invalid arguments for plpy.prepare");
return NULL;
}
if (list && (!PySequence_Check(list)))
{
- PLy_exception_set(PLy_exc_spi_error,
+ PLy_exception_set(PyExc_ValueError,
"second argument of plpy.prepare must be a sequence");
return NULL;
}
@@ -3004,7 +3002,7 @@ PLy_spi_execute_plan(PyObject *ob, PyObject *list, long limit)
{
if (!PySequence_Check(list) || PyString_Check(list) || PyUnicode_Check(list))
{
- PLy_exception_set(PLy_exc_spi_error, "plpy.execute takes a sequence as its second argument");
+ PLy_exception_set(PyExc_ValueError, "plpy.execute takes a sequence as its second argument");
return NULL;
}
nargs = PySequence_Length(list);
@@ -3022,9 +3020,9 @@ PLy_spi_execute_plan(PyObject *ob, PyObject *list, long limit)
if (so == NULL)
PLy_elog(ERROR, "could not execute plan");
sv = PyString_AsString(so);
- PLy_exception_set_plural(PLy_exc_spi_error,
- "Expected sequence of %d argument, got %d: %s",
- "Expected sequence of %d arguments, got %d: %s",
+ PLy_exception_set_plural(PyExc_ValueError,
+ "Expected sequence of %d argument, got %d: %s",
+ "Expected sequence of %d arguments, got %d: %s",
plan->nargs,
plan->nargs, nargs, sv);
Py_DECREF(so);
@@ -3109,7 +3107,7 @@ PLy_spi_execute_plan(PyObject *ob, PyObject *list, long limit)
}
if (!PyErr_Occurred())
- PLy_exception_set(PLy_exc_error,
+ PLy_exception_set(PLy_exc_spi_error,
"unrecognized error in PLy_spi_execute_plan");
PLy_elog(WARNING, NULL);
PLy_spi_exception_set(edata);
--
1.7.2.3
>From 45294a76be7b32da5dc006286eeb6ca0e3ee63a1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Urba=C5=84ski?= <wulc...@wulczer.org>
Date: Sun, 2 Jan 2011 11:32:57 +0100
Subject: [PATCH 5/7] Call PLy_spi_execute_fetch_result inside the try/catch block.
This way errors from fetching tuples are correctly reported as errors
in the SPI call. While at it, ged rid of a useless loop variable, do
not pfree the result of a possible palloc(0) and do some minor
stylistic changes.
---
src/pl/plpython/plpython.c | 49 +++++++++++++++++++++++++------------------
1 files changed, 28 insertions(+), 21 deletions(-)
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index 29d6866..1eeabb0 100644
--- a/src/pl/plpython/plpython.c
+++ b/src/pl/plpython/plpython.c
@@ -2994,11 +2994,11 @@ PLy_spi_execute(PyObject *self, PyObject *args)
static PyObject *
PLy_spi_execute_plan(PyObject *ob, PyObject *list, long limit)
{
- volatile int nargs;
- int i,
- rv;
- PLyPlanObject *plan;
- volatile MemoryContext oldcontext;
+ int volatile nargs;
+ int i, rv;
+ PLyPlanObject *plan;
+ PyObject *ret;
+ MemoryContext volatile oldcontext = CurrentMemoryContext;
if (list != NULL)
{
@@ -3019,7 +3019,7 @@ PLy_spi_execute_plan(PyObject *ob, PyObject *list, long limit)
char *sv;
PyObject *so = PyObject_Str(list);
- if (!so)
+ if (so == NULL)
PLy_elog(ERROR, "could not execute plan");
sv = PyString_AsString(so);
PLy_exception_set_plural(PLy_exc_spi_error,
@@ -3032,11 +3032,15 @@ PLy_spi_execute_plan(PyObject *ob, PyObject *list, long limit)
return NULL;
}
- oldcontext = CurrentMemoryContext;
PG_TRY();
{
- char *nulls = palloc(nargs * sizeof(char));
- volatile int j;
+ char *nulls;
+ int volatile j;
+
+ if (nargs > 0)
+ nulls = palloc(nargs * sizeof(char));
+ else
+ nulls = NULL;
for (j = 0; j < nargs; j++)
{
@@ -3076,12 +3080,15 @@ PLy_spi_execute_plan(PyObject *ob, PyObject *list, long limit)
rv = SPI_execute_plan(plan->plan, plan->values, nulls,
PLy_curr_procedure->fn_readonly, limit);
+ ret = PLy_spi_execute_fetch_result(SPI_tuptable, SPI_processed, rv);
+
+ if (nargs > 0)
+ pfree(nulls);
+
- pfree(nulls);
}
PG_CATCH();
{
- int k;
ErrorData *edata;
MemoryContextSwitchTo(oldcontext);
@@ -3091,13 +3098,13 @@ PLy_spi_execute_plan(PyObject *ob, PyObject *list, long limit)
/*
* cleanup plan->values array
*/
- for (k = 0; k < nargs; k++)
+ for (i = 0; i < nargs; i++)
{
- if (!plan->args[k].out.d.typbyval &&
- (plan->values[k] != PointerGetDatum(NULL)))
+ if (!plan->args[i].out.d.typbyval &&
+ (plan->values[i] != PointerGetDatum(NULL)))
{
- pfree(DatumGetPointer(plan->values[k]));
- plan->values[k] = PointerGetDatum(NULL);
+ pfree(DatumGetPointer(plan->values[i]));
+ plan->values[i] = PointerGetDatum(NULL);
}
}
@@ -3120,21 +3127,21 @@ PLy_spi_execute_plan(PyObject *ob, PyObject *list, long limit)
}
}
-
- return PLy_spi_execute_fetch_result(SPI_tuptable, SPI_processed, rv);
+ return ret;
}
static PyObject *
PLy_spi_execute_query(char *query, long limit)
{
int rv;
- volatile MemoryContext oldcontext;
+ PyObject *ret;
+ MemoryContext volatile oldcontext = CurrentMemoryContext;
- 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();
{
@@ -3160,7 +3167,7 @@ PLy_spi_execute_query(char *query, long limit)
return NULL;
}
- return PLy_spi_execute_fetch_result(SPI_tuptable, SPI_processed, rv);
+ return ret;
}
static PyObject *
--
1.7.2.3
>From 6348ae2c5fe532d9fd21d87d47677b6bad47fc7f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Urba=C5=84ski?= <wulc...@wulczer.org>
Date: Sun, 2 Jan 2011 11:29:06 +0100
Subject: [PATCH 4/7] Refactor PLy_spi_prepare to save two levels of indentation.
Instead of checking if the arglist is NULL and then if its length is
0, do it in one step, and outside of the try/catch block.
---
src/pl/plpython/plpython.c | 135 ++++++++++++++++++++++----------------------
1 files changed, 67 insertions(+), 68 deletions(-)
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index be39799..29d6866 100644
--- a/src/pl/plpython/plpython.c
+++ b/src/pl/plpython/plpython.c
@@ -2838,8 +2838,9 @@ PLy_spi_prepare(PyObject *self, PyObject *args)
PyObject *volatile optr = NULL;
char *query;
void *tmpplan;
- volatile MemoryContext oldcontext;
+ int nargs;
+ MemoryContext volatile oldcontext = CurrentMemoryContext;
if (!PyArg_ParseTuple(args, "s|O", &query, &list))
{
@@ -2858,80 +2859,77 @@ PLy_spi_prepare(PyObject *self, PyObject *args)
if ((plan = (PLyPlanObject *) PLy_plan_new()) == NULL)
return NULL;
- oldcontext = CurrentMemoryContext;
+ nargs = (list == NULL ? 0 : PySequence_Length(list));
+
+ plan->nargs = nargs;
+ plan->types = PLy_malloc(sizeof(Oid) * nargs);
+ plan->values = PLy_malloc(sizeof(Datum) * nargs);
+ plan->args = PLy_malloc(sizeof(PLyTypeInfo) * nargs);
+
PG_TRY();
{
- if (list != NULL)
+ int i;
+
+ /*
+ * the other loop might throw an exception, if PLyTypeInfo
+ * member isn't properly initialized the Py_DECREF(plan) will
+ * go boom
+ */
+ for (i = 0; i < nargs; i++)
{
- int nargs,
- i;
+ PLy_typeinfo_init(&plan->args[i]);
+ plan->values[i] = PointerGetDatum(NULL);
+ }
- nargs = PySequence_Length(list);
- if (nargs > 0)
+ for (i = 0; i < nargs; i++)
+ {
+ char *sptr;
+ HeapTuple typeTup;
+ Oid typeId;
+ int32 typmod;
+ Form_pg_type typeStruct;
+
+ optr = PySequence_GetItem(list, i);
+ if (PyString_Check(optr))
+ sptr = PyString_AsString(optr);
+ else if (PyUnicode_Check(optr))
+ sptr = PLyUnicode_AsString(optr);
+ else
{
- plan->nargs = nargs;
- plan->types = PLy_malloc(sizeof(Oid) * nargs);
- plan->values = PLy_malloc(sizeof(Datum) * nargs);
- plan->args = PLy_malloc(sizeof(PLyTypeInfo) * nargs);
+ ereport(ERROR,
+ (errmsg("plpy.prepare: type name at ordinal position %d is not a string", i)));
+ sptr = NULL; /* keep compiler quiet */
+ }
- /*
- * the other loop might throw an exception, if PLyTypeInfo
- * member isn't properly initialized the Py_DECREF(plan) will
- * go boom
- */
- for (i = 0; i < nargs; i++)
- {
- PLy_typeinfo_init(&plan->args[i]);
- plan->values[i] = PointerGetDatum(NULL);
- }
+ /********************************************************
+ * Resolve argument type names and then look them up by
+ * oid in the system cache, and remember the required
+ *information for input conversion.
+ ********************************************************/
- for (i = 0; i < nargs; i++)
- {
- char *sptr;
- HeapTuple typeTup;
- Oid typeId;
- int32 typmod;
- Form_pg_type typeStruct;
-
- optr = PySequence_GetItem(list, i);
- if (PyString_Check(optr))
- sptr = PyString_AsString(optr);
- else if (PyUnicode_Check(optr))
- sptr = PLyUnicode_AsString(optr);
- else
- {
- ereport(ERROR,
- (errmsg("plpy.prepare: type name at ordinal position %d is not a string", i)));
- sptr = NULL; /* keep compiler quiet */
- }
-
- /********************************************************
- * Resolve argument type names and then look them up by
- * oid in the system cache, and remember the required
- *information for input conversion.
- ********************************************************/
-
- parseTypeString(sptr, &typeId, &typmod);
-
- typeTup = SearchSysCache1(TYPEOID,
- ObjectIdGetDatum(typeId));
- if (!HeapTupleIsValid(typeTup))
- elog(ERROR, "cache lookup failed for type %u", typeId);
-
- Py_DECREF(optr);
- optr = NULL; /* this is important */
-
- plan->types[i] = typeId;
- typeStruct = (Form_pg_type) GETSTRUCT(typeTup);
- if (typeStruct->typtype != TYPTYPE_COMPOSITE)
- PLy_output_datum_func(&plan->args[i], typeTup);
- else
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("plpy.prepare does not support composite types")));
- ReleaseSysCache(typeTup);
- }
- }
+ parseTypeString(sptr, &typeId, &typmod);
+
+ typeTup = SearchSysCache1(TYPEOID,
+ ObjectIdGetDatum(typeId));
+ if (!HeapTupleIsValid(typeTup))
+ elog(ERROR, "cache lookup failed for type %u", typeId);
+
+ Py_DECREF(optr);
+ /*
+ * set optr to NULL, so we won't try to unref it again in
+ * case of an error
+ */
+ optr = NULL;
+
+ plan->types[i] = typeId;
+ typeStruct = (Form_pg_type) GETSTRUCT(typeTup);
+ if (typeStruct->typtype != TYPTYPE_COMPOSITE)
+ PLy_output_datum_func(&plan->args[i], typeTup);
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("plpy.prepare does not support composite types")));
+ ReleaseSysCache(typeTup);
}
pg_verifymbstr(query, strlen(query), false);
@@ -2965,6 +2963,7 @@ PLy_spi_prepare(PyObject *self, PyObject *args)
}
PG_END_TRY();
+ Assert(plan->plan != NULL);
return (PyObject *) plan;
}
--
1.7.2.3
>From 8dcda806e6dd529c41196a5ae32c5f14aab3f6fe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Urba=C5=84ski?= <wulc...@wulczer.org>
Date: Sun, 2 Jan 2011 11:21:30 +0100
Subject: [PATCH 3/7] Correctly add exceptions to the plpy module for Python 3.
The way the exception types where added to the module was wrong for
Python 3. Exception classes were not actually available from plpy. Fix
that by factoring out code that is responsible for defining new Python
exceptions and make it work with Python 3. New regression test to make
sure the plpy module has the expected contents.
---
src/pl/plpython/expected/plpython_test.out | 13 ++++++
src/pl/plpython/plpython.c | 63 +++++++++++++++++++++++-----
src/pl/plpython/sql/plpython_test.sql | 9 ++++
3 files changed, 74 insertions(+), 11 deletions(-)
diff --git a/src/pl/plpython/expected/plpython_test.out b/src/pl/plpython/expected/plpython_test.out
index df463c6..583a149 100644
--- a/src/pl/plpython/expected/plpython_test.out
+++ b/src/pl/plpython/expected/plpython_test.out
@@ -35,6 +35,19 @@ select argument_test_one(users, fname, lname) from users where lname = 'doe' ord
willem doe => {fname: willem, lname: doe, userid: 3, username: w_doe}
(3 rows)
+-- check module contents
+CREATE FUNCTION module_contents() RETURNS text AS
+$$
+contents = list(filter(lambda x: not x.startswith("__"), dir(plpy)))
+contents.sort()
+return ", ".join(contents)
+$$ LANGUAGE plpythonu;
+select module_contents();
+ module_contents
+-------------------------------------------------------------------------------------------
+ Error, Fatal, SPIError, debug, error, execute, fatal, info, log, notice, prepare, warning
+(1 row)
+
CREATE FUNCTION elog_test() RETURNS void
AS $$
plpy.debug('debug')
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index f205768..be39799 100644
--- a/src/pl/plpython/plpython.c
+++ b/src/pl/plpython/plpython.c
@@ -3226,6 +3226,46 @@ PLy_spi_execute_fetch_result(SPITupleTable *tuptable, int rows, int status)
return (PyObject *) result;
}
+/*
+ * Exception support
+ */
+
+/*
+ * Add an exception to the module. The first parameter is either the actual
+ * module (for Python 3) or the module dictionary
+ */
+static void PLy_add_exception(PyObject *mod, const char *name, PyObject *exc)
+{
+#if PY_MAJOR_VERSION >= 3
+ Py_INCREF(exc);
+ PyModule_AddObject(mod, name, exc);
+#else
+ PyDict_SetItemString(mod, name, exc);
+#endif
+}
+
+/* Add exceptions to the plpy module */
+static void
+PLy_initialize_exceptions(PyObject *plpy)
+{
+ PyObject *mod;
+
+#if PY_MAJOR_VERSION < 3
+ /* For Python <3 we add the exceptions to the module dictionary */
+ mod = PyModule_GetDict(plpy);
+#else
+ /* In Python 3 you add them directly into the module */
+ mod = plpy;
+#endif
+
+ PLy_exc_error = PyErr_NewException("plpy.Error", NULL, NULL);
+ PLy_exc_fatal = PyErr_NewException("plpy.Fatal", NULL, NULL);
+ PLy_exc_spi_error = PyErr_NewException("plpy.SPIError", NULL, NULL);
+
+ PLy_add_exception(mod, "Error", PLy_exc_error);
+ PLy_add_exception(mod, "Fatal", PLy_exc_fatal);
+ PLy_add_exception(mod, "SPIError", PLy_exc_spi_error);
+}
/*
* language handler and interpreter initialization
@@ -3235,7 +3275,15 @@ PLy_spi_execute_fetch_result(SPITupleTable *tuptable, int rows, int status)
static PyMODINIT_FUNC
PyInit_plpy(void)
{
- return PyModule_Create(&PLy_module);
+ PyObject *m;
+
+ m = PyModule_Create(&PLy_module);
+ if (m == NULL)
+ return NULL;
+
+ PLy_initialize_exceptions(m);
+
+ return m;
}
#endif
@@ -3326,8 +3374,7 @@ PLy_init_plpy(void)
PyObject *main_mod,
*main_dict,
*plpy_mod;
- PyObject *plpy,
- *plpy_dict;
+ PyObject *plpy;
/*
* initialize plpy module
@@ -3341,18 +3388,12 @@ PLy_init_plpy(void)
plpy = PyModule_Create(&PLy_module);
#else
plpy = Py_InitModule("plpy", PLy_methods);
+ /* for Python 3 we initialized the exceptions in PyInit_plpy */
+ PLy_initialize_exceptions(plpy);
#endif
- plpy_dict = PyModule_GetDict(plpy);
/* PyDict_SetItemString(plpy, "PlanType", (PyObject *) &PLy_PlanType); */
- PLy_exc_error = PyErr_NewException("plpy.Error", NULL, NULL);
- PLy_exc_fatal = PyErr_NewException("plpy.Fatal", NULL, NULL);
- PLy_exc_spi_error = PyErr_NewException("plpy.SPIError", NULL, NULL);
- PyDict_SetItemString(plpy_dict, "Error", PLy_exc_error);
- PyDict_SetItemString(plpy_dict, "Fatal", PLy_exc_fatal);
- PyDict_SetItemString(plpy_dict, "SPIError", PLy_exc_spi_error);
-
/*
* initialize main module, and add plpy
*/
diff --git a/src/pl/plpython/sql/plpython_test.sql b/src/pl/plpython/sql/plpython_test.sql
index 7cae124..298411a 100644
--- a/src/pl/plpython/sql/plpython_test.sql
+++ b/src/pl/plpython/sql/plpython_test.sql
@@ -25,6 +25,15 @@ return words'
select argument_test_one(users, fname, lname) from users where lname = 'doe' order by 1;
+-- check module contents
+CREATE FUNCTION module_contents() RETURNS text AS
+$$
+contents = list(filter(lambda x: not x.startswith("__"), dir(plpy)))
+contents.sort()
+return ", ".join(contents)
+$$ LANGUAGE plpythonu;
+
+select module_contents();
CREATE FUNCTION elog_test() RETURNS void
AS $$
--
1.7.2.3
>From 0eba6c4ed89b1815ae28cc34db74f9c858d253ca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Urba=C5=84ski?= <wulc...@wulczer.org>
Date: Wed, 19 Jan 2011 23:05:40 +0100
Subject: [PATCH 2/7] Factor out functions responsible for caching I/O routines.
This makes PLy_procedure_create a bit more manageable.
---
src/pl/plpython/plpython.c | 241 +++++++++++++++++++++++---------------------
1 files changed, 128 insertions(+), 113 deletions(-)
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index bed5e70..f205768 100644
--- a/src/pl/plpython/plpython.c
+++ b/src/pl/plpython/plpython.c
@@ -1367,6 +1367,132 @@ PLy_procedure_get(Oid fn_oid, bool is_trigger)
return entry->proc;
}
+/* Set up output conversion functions for a procedure */
+static void
+PLy_procedure_output_conversion(PLyProcedure *proc, Form_pg_proc procStruct)
+{
+ HeapTuple rvTypeTup;
+ Form_pg_type rvTypeStruct;
+
+ /* Get the return type */
+ rvTypeTup = SearchSysCache1(TYPEOID,
+ ObjectIdGetDatum(procStruct->prorettype));
+ if (!HeapTupleIsValid(rvTypeTup))
+ elog(ERROR, "cache lookup failed for type %u",
+ procStruct->prorettype);
+ rvTypeStruct = (Form_pg_type) GETSTRUCT(rvTypeTup);
+
+ /* Disallow pseudotype result, except for void */
+ if (rvTypeStruct->typtype == TYPTYPE_PSEUDO &&
+ procStruct->prorettype != VOIDOID)
+ {
+ if (procStruct->prorettype == TRIGGEROID)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("trigger functions can only be called as triggers")));
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("PL/Python functions cannot return type %s",
+ format_type_be(procStruct->prorettype))));
+ }
+
+ if (rvTypeStruct->typtype == TYPTYPE_COMPOSITE)
+ {
+ /*
+ * Tuple: set up later, during first call to
+ * PLy_function_handler
+ */
+ proc->result.out.d.typoid = procStruct->prorettype;
+ proc->result.is_rowtype = 2;
+ }
+ else
+ {
+ /* Do the real work */
+ PLy_output_datum_func(&proc->result, rvTypeTup);
+ }
+
+ ReleaseSysCache(rvTypeTup);
+}
+
+/* Set up output conversion functions for a procedure */
+static void
+PLy_procedure_input_conversion(PLyProcedure *proc, HeapTuple procTup,
+ Form_pg_proc procStruct)
+{
+ Oid *types;
+ char **names,
+ *modes;
+ int i,
+ pos,
+ total;
+
+ /* Extract argument type info from the pg_proc tuple */
+ total = get_func_arg_info(procTup, &types, &names, &modes);
+
+ /* Count number of in+inout args into proc->nargs */
+ if (modes == NULL)
+ proc->nargs = total;
+ else
+ {
+ /* proc->nargs was initialized to 0 above */
+ for (i = 0; i < total; i++)
+ {
+ if (modes[i] != PROARGMODE_OUT &&
+ modes[i] != PROARGMODE_TABLE)
+ (proc->nargs)++;
+ }
+ }
+
+ proc->argnames = (char **) PLy_malloc0(sizeof(char *) * proc->nargs);
+ for (i = pos = 0; i < total; i++)
+ {
+ HeapTuple argTypeTup;
+ Form_pg_type argTypeStruct;
+
+ if (modes &&
+ (modes[i] == PROARGMODE_OUT ||
+ modes[i] == PROARGMODE_TABLE))
+ continue; /* skip OUT arguments */
+
+ Assert(types[i] == procStruct->proargtypes.values[pos]);
+
+ argTypeTup = SearchSysCache1(TYPEOID,
+ ObjectIdGetDatum(types[i]));
+ if (!HeapTupleIsValid(argTypeTup))
+ elog(ERROR, "cache lookup failed for type %u", types[i]);
+ argTypeStruct = (Form_pg_type) GETSTRUCT(argTypeTup);
+
+ /* Check argument type is OK, set up I/O function info */
+ switch (argTypeStruct->typtype)
+ {
+ case TYPTYPE_PSEUDO:
+ /* Disallow pseudotype argument */
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("PL/Python functions cannot accept type %s",
+ format_type_be(types[i]))));
+ break;
+ case TYPTYPE_COMPOSITE:
+ /* We'll set IO funcs at first call */
+ proc->args[pos].is_rowtype = 2;
+ break;
+ default:
+ PLy_input_datum_func(&(proc->args[pos]),
+ types[i],
+ argTypeTup);
+ break;
+ }
+
+ /* Get argument name */
+ proc->argnames[pos] = names ? PLy_strdup(names[i]) : NULL;
+
+ ReleaseSysCache(argTypeTup);
+
+ pos++;
+ }
+}
+
/*
* Create a new PLyProcedure structure
*/
@@ -1415,46 +1541,7 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger)
* but only if this isn't a trigger.
*/
if (!is_trigger)
- {
- HeapTuple rvTypeTup;
- Form_pg_type rvTypeStruct;
-
- rvTypeTup = SearchSysCache1(TYPEOID,
- ObjectIdGetDatum(procStruct->prorettype));
- if (!HeapTupleIsValid(rvTypeTup))
- elog(ERROR, "cache lookup failed for type %u",
- procStruct->prorettype);
- rvTypeStruct = (Form_pg_type) GETSTRUCT(rvTypeTup);
-
- /* Disallow pseudotype result, except for void */
- if (rvTypeStruct->typtype == TYPTYPE_PSEUDO &&
- procStruct->prorettype != VOIDOID)
- {
- if (procStruct->prorettype == TRIGGEROID)
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("trigger functions can only be called as triggers")));
- else
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("PL/Python functions cannot return type %s",
- format_type_be(procStruct->prorettype))));
- }
-
- if (rvTypeStruct->typtype == TYPTYPE_COMPOSITE)
- {
- /*
- * Tuple: set up later, during first call to
- * PLy_function_handler
- */
- proc->result.out.d.typoid = procStruct->prorettype;
- proc->result.is_rowtype = 2;
- }
- else
- PLy_output_datum_func(&proc->result, rvTypeTup);
-
- ReleaseSysCache(rvTypeTup);
- }
+ PLy_procedure_output_conversion(proc, procStruct);
/*
* Now get information required for input conversion of the
@@ -1464,79 +1551,7 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger)
* arguments.
*/
if (procStruct->pronargs)
- {
- Oid *types;
- char **names,
- *modes;
- int i,
- pos,
- total;
-
- /* extract argument type info from the pg_proc tuple */
- total = get_func_arg_info(procTup, &types, &names, &modes);
-
- /* count number of in+inout args into proc->nargs */
- if (modes == NULL)
- proc->nargs = total;
- else
- {
- /* proc->nargs was initialized to 0 above */
- for (i = 0; i < total; i++)
- {
- if (modes[i] != PROARGMODE_OUT &&
- modes[i] != PROARGMODE_TABLE)
- (proc->nargs)++;
- }
- }
-
- proc->argnames = (char **) PLy_malloc0(sizeof(char *) * proc->nargs);
- for (i = pos = 0; i < total; i++)
- {
- HeapTuple argTypeTup;
- Form_pg_type argTypeStruct;
-
- if (modes &&
- (modes[i] == PROARGMODE_OUT ||
- modes[i] == PROARGMODE_TABLE))
- continue; /* skip OUT arguments */
-
- Assert(types[i] == procStruct->proargtypes.values[pos]);
-
- argTypeTup = SearchSysCache1(TYPEOID,
- ObjectIdGetDatum(types[i]));
- if (!HeapTupleIsValid(argTypeTup))
- elog(ERROR, "cache lookup failed for type %u", types[i]);
- argTypeStruct = (Form_pg_type) GETSTRUCT(argTypeTup);
-
- /* check argument type is OK, set up I/O function info */
- switch (argTypeStruct->typtype)
- {
- case TYPTYPE_PSEUDO:
- /* Disallow pseudotype argument */
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("PL/Python functions cannot accept type %s",
- format_type_be(types[i]))));
- break;
- case TYPTYPE_COMPOSITE:
- /* we'll set IO funcs at first call */
- proc->args[pos].is_rowtype = 2;
- break;
- default:
- PLy_input_datum_func(&(proc->args[pos]),
- types[i],
- argTypeTup);
- break;
- }
-
- /* get argument name */
- proc->argnames[pos] = names ? PLy_strdup(names[i]) : NULL;
-
- ReleaseSysCache(argTypeTup);
-
- pos++;
- }
- }
+ PLy_procedure_input_conversion(proc, procTup, procStruct);
/*
* get the text of the function.
--
1.7.2.3
>From 1f3ff204530ab48826230329e70303e4f6a3dc93 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Urba=C5=84ski?= <wulc...@wulczer.org>
Date: Sat, 1 Jan 2011 22:48:09 +0100
Subject: [PATCH 1/7] Get rid of the global variable holding the error state.
Global error handling lead to confusion and was hard to manage. With
this change, errors from PostgreSQL are immediately reported to Python
as exceptions. This requires setting a Python exception after
reporting the caught Postgres error as a warning, because PLy_elog
destroys the Python exception state. Ideally, all places where
Postgres errors need to be reported back to Python should be wrapped
in subtransactions, to make going back to Python from a longjmp
safe. This will be handled in a separate patch.
---
src/pl/plpython/expected/plpython_error.out | 15 ++-
src/pl/plpython/expected/plpython_test.out | 2 +-
src/pl/plpython/plpython.c | 167 +++++++++++++++++----------
3 files changed, 117 insertions(+), 67 deletions(-)
diff --git a/src/pl/plpython/expected/plpython_error.out b/src/pl/plpython/expected/plpython_error.out
index 1f24c13..87225f2 100644
--- a/src/pl/plpython/expected/plpython_error.out
+++ b/src/pl/plpython/expected/plpython_error.out
@@ -10,7 +10,7 @@ CREATE FUNCTION sql_syntax_error() RETURNS text
SELECT sql_syntax_error();
WARNING: PL/Python: plpy.SPIError: unrecognized error in PLy_spi_execute_query
CONTEXT: PL/Python function "sql_syntax_error"
-ERROR: syntax error at or near "syntax"
+ERROR: PL/Python: plpy.SPIError: syntax error at or near "syntax"
LINE 1: syntax error
^
QUERY: syntax error
@@ -34,7 +34,7 @@ return rv[0]'
SELECT exception_index_invalid_nested();
WARNING: PL/Python: plpy.SPIError: unrecognized error in PLy_spi_execute_query
CONTEXT: PL/Python function "exception_index_invalid_nested"
-ERROR: function test5(unknown) does not exist
+ERROR: PL/Python: plpy.SPIError: function test5(unknown) does not exist
LINE 1: SELECT test5('foo')
^
HINT: No function matches the given name and argument types. You might need to add explicit type casts.
@@ -56,7 +56,7 @@ return None
SELECT invalid_type_uncaught('rick');
WARNING: PL/Python: plpy.SPIError: unrecognized error in PLy_spi_prepare
CONTEXT: PL/Python function "invalid_type_uncaught"
-ERROR: type "test" does not exist
+ERROR: PL/Python: plpy.SPIError: type "test" does not exist
CONTEXT: PL/Python function "invalid_type_uncaught"
/* for what it's worth catch the exception generated by
* the typo, and return None
@@ -79,8 +79,13 @@ return None
SELECT invalid_type_caught('rick');
WARNING: PL/Python: plpy.SPIError: unrecognized error in PLy_spi_prepare
CONTEXT: PL/Python function "invalid_type_caught"
-ERROR: type "test" does not exist
+NOTICE: type "test" does not exist
CONTEXT: PL/Python function "invalid_type_caught"
+ invalid_type_caught
+---------------------
+
+(1 row)
+
/* for what it's worth catch the exception generated by
* the typo, and reraise it as a plain error
*/
@@ -101,7 +106,7 @@ return None
SELECT invalid_type_reraised('rick');
WARNING: PL/Python: plpy.SPIError: unrecognized error in PLy_spi_prepare
CONTEXT: PL/Python function "invalid_type_reraised"
-ERROR: type "test" does not exist
+ERROR: PL/Python: plpy.Error: type "test" does not exist
CONTEXT: PL/Python function "invalid_type_reraised"
/* no typo no messing about
*/
diff --git a/src/pl/plpython/expected/plpython_test.out b/src/pl/plpython/expected/plpython_test.out
index a229b18..df463c6 100644
--- a/src/pl/plpython/expected/plpython_test.out
+++ b/src/pl/plpython/expected/plpython_test.out
@@ -60,5 +60,5 @@ NOTICE: notice
CONTEXT: PL/Python function "elog_test"
WARNING: warning
CONTEXT: PL/Python function "elog_test"
-ERROR: error
+ERROR: PL/Python: plpy.Error: error
CONTEXT: PL/Python function "elog_test"
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index d083c6e..bed5e70 100644
--- a/src/pl/plpython/plpython.c
+++ b/src/pl/plpython/plpython.c
@@ -284,6 +284,10 @@ PLy_exception_set_plural(PyObject *, const char *, const char *,
__attribute__((format(printf, 2, 5)))
__attribute__((format(printf, 3, 5)));
+/* like PLy_exception_set, but conserve more fields from ErrorData */
+static void
+PLy_spi_exception_set(ErrorData *edata);
+
/* Get the innermost python procedure called from the backend */
static char *PLy_procedure_name(PLyProcedure *);
@@ -291,6 +295,7 @@ static char *PLy_procedure_name(PLyProcedure *);
static void
PLy_elog(int, const char *,...)
__attribute__((format(printf, 2, 3)));
+static void PLy_spi_error_data(PyObject *, char **, char **, int *);
static char *PLy_traceback(int *);
static void *PLy_malloc(size_t);
@@ -364,18 +369,6 @@ static HeapTuple PLyObject_ToTuple(PLyTypeInfo *, PyObject *);
*/
static PLyProcedure *PLy_curr_procedure = NULL;
-/*
- * When a callback from Python into PG incurs an error, we temporarily store
- * the error information here, and return NULL to the Python interpreter.
- * Any further callback attempts immediately fail, and when the Python
- * interpreter returns to the calling function, we re-throw the error (even if
- * Python thinks it trapped the error and doesn't return NULL). Eventually
- * this ought to be improved to let Python code really truly trap the error,
- * but that's more of a change from the pre-8.0 semantics than I have time for
- * now --- it will only be possible if the callback query is executed inside a
- * subtransaction.
- */
-static ErrorData *PLy_error_in_progress = NULL;
static PyObject *PLy_interp_globals = NULL;
static PyObject *PLy_interp_safe_globals = NULL;
@@ -597,7 +590,6 @@ PLy_trigger_handler(FunctionCallInfo fcinfo, PLyProcedure *proc)
plrv = PLy_procedure_call(proc, "TD", plargs);
Assert(plrv != NULL);
- Assert(!PLy_error_in_progress);
/*
* Disconnect from SPI manager
@@ -1015,7 +1007,6 @@ PLy_function_handler(FunctionCallInfo fcinfo, PLyProcedure *proc)
PLy_function_delete_args(proc);
}
Assert(plrv != NULL);
- Assert(!PLy_error_in_progress);
}
/*
@@ -1198,19 +1189,10 @@ PLy_procedure_call(PLyProcedure *proc, char *kargs, PyObject *vargs)
* If there was an error in a PG callback, propagate that no matter what
* Python claims about its success.
*/
- if (PLy_error_in_progress)
- {
- ErrorData *edata = PLy_error_in_progress;
-
- PLy_error_in_progress = NULL;
- ReThrowError(edata);
- }
- if (rv == NULL || PyErr_Occurred())
- {
- Py_XDECREF(rv);
+ /* if the Python code returned an error, propagate it */
+ if (rv == NULL)
PLy_elog(ERROR, NULL);
- }
return rv;
}
@@ -2843,12 +2825,6 @@ PLy_spi_prepare(PyObject *self, PyObject *args)
void *tmpplan;
volatile MemoryContext oldcontext;
- /* Can't execute more if we have an unhandled error */
- if (PLy_error_in_progress)
- {
- PLy_exception_set(PLy_exc_error, "transaction aborted");
- return NULL;
- }
if (!PyArg_ParseTuple(args, "s|O", &query, &list))
{
@@ -2959,15 +2935,17 @@ PLy_spi_prepare(PyObject *self, PyObject *args)
}
PG_CATCH();
{
+ ErrorData *edata;
+
MemoryContextSwitchTo(oldcontext);
- PLy_error_in_progress = CopyErrorData();
- FlushErrorState();
+ edata = CopyErrorData();
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;
}
PG_END_TRY();
@@ -2986,13 +2964,6 @@ PLy_spi_execute(PyObject *self, PyObject *args)
PyObject *list = NULL;
long limit = 0;
- /* Can't execute more if we have an unhandled error */
- if (PLy_error_in_progress)
- {
- PLy_exception_set(PLy_exc_error, "transaction aborted");
- return NULL;
- }
-
if (PyArg_ParseTuple(args, "s|l", &query, &limit))
return PLy_spi_execute_query(query, limit);
@@ -3097,9 +3068,10 @@ PLy_spi_execute_plan(PyObject *ob, PyObject *list, long limit)
PG_CATCH();
{
int k;
+ ErrorData *edata;
MemoryContextSwitchTo(oldcontext);
- PLy_error_in_progress = CopyErrorData();
+ edata = CopyErrorData();
FlushErrorState();
/*
@@ -3119,6 +3091,7 @@ PLy_spi_execute_plan(PyObject *ob, PyObject *list, long limit)
PLy_exception_set(PLy_exc_error,
"unrecognized error in PLy_spi_execute_plan");
PLy_elog(WARNING, NULL);
+ PLy_spi_exception_set(edata);
return NULL;
}
PG_END_TRY();
@@ -3133,13 +3106,6 @@ PLy_spi_execute_plan(PyObject *ob, PyObject *list, long limit)
}
}
- if (rv < 0)
- {
- PLy_exception_set(PLy_exc_spi_error,
- "SPI_execute_plan failed: %s",
- SPI_result_code_string(rv));
- return NULL;
- }
return PLy_spi_execute_fetch_result(SPI_tuptable, SPI_processed, rv);
}
@@ -3158,13 +3124,16 @@ PLy_spi_execute_query(char *query, long limit)
}
PG_CATCH();
{
+ ErrorData *edata;
+
MemoryContextSwitchTo(oldcontext);
- PLy_error_in_progress = CopyErrorData();
+ 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;
}
PG_END_TRY();
@@ -3225,8 +3194,6 @@ PLy_spi_execute_fetch_result(SPITupleTable *tuptable, int rows, int status)
PG_CATCH();
{
MemoryContextSwitchTo(oldcontext);
- PLy_error_in_progress = CopyErrorData();
- FlushErrorState();
if (!PyErr_Occurred())
PLy_exception_set(PLy_exc_error,
"unrecognized error in PLy_spi_execute_fetch_result");
@@ -3464,23 +3431,21 @@ PLy_output(volatile int level, PyObject *self, PyObject *args)
}
PG_CATCH();
{
+ ErrorData *edata;
+
+ /* Must reset elog.c's state */
MemoryContextSwitchTo(oldcontext);
- PLy_error_in_progress = CopyErrorData();
+ edata = CopyErrorData();
FlushErrorState();
- PyErr_SetString(PLy_exc_error, sv);
-
/*
* Note: If sv came from PyString_AsString(), it points into storage
* owned by so. So free so after using sv.
*/
Py_XDECREF(so);
- /*
- * returning NULL here causes the python interpreter to bail. when
- * control passes back to PLy_procedure_call, we check for PG
- * exceptions and re-throw the error.
- */
+ /* Make Python raise the exception */
+ PLy_exception_set(PLy_exc_error, edata->message);
return NULL;
}
PG_END_TRY();
@@ -3546,6 +3511,49 @@ PLy_exception_set_plural(PyObject *exc,
PyErr_SetString(exc, buf);
}
+/*
+ * Raise a SPIError, passing in it more error details, like the internal query
+ * and error position.
+ */
+static void
+PLy_spi_exception_set(ErrorData *edata)
+{
+ PyObject *args = NULL;
+ PyObject *spierror = NULL;
+ PyObject *spidata = NULL;
+
+ args = Py_BuildValue("(s)", edata->message);
+ if (!args)
+ goto failure;
+
+ /* create a new SPIError with the error message as the parameter */
+ spierror = PyObject_CallObject(PLy_exc_spi_error, args);
+ if (!spierror)
+ goto failure;
+
+ spidata = Py_BuildValue("(zzi)", edata->hint,
+ edata->internalquery, edata->internalpos);
+ if (!spidata)
+ goto failure;
+
+ if (PyObject_SetAttrString(spierror, "spidata", spidata) == -1)
+ goto failure;
+
+ PyErr_SetObject(PLy_exc_spi_error, spierror);
+
+ Py_DECREF(args);
+ Py_DECREF(spierror);
+ Py_DECREF(spidata);
+ return;
+
+failure:
+ Py_XDECREF(args);
+ Py_XDECREF(spierror);
+ Py_XDECREF(spidata);
+ elog(ERROR, "could not convert SPI error to Python exception");
+}
+
+
/* Emit a PG error or notice, together with any available info about
* the current Python error, previously set by PLy_exception_set().
* This should be used to propagate Python errors into PG. If fmt is
@@ -3558,6 +3566,15 @@ PLy_elog(int elevel, const char *fmt,...)
char *xmsg;
int xlevel;
StringInfoData emsg;
+ PyObject *e, *v, *tb;
+ char *hint = NULL;
+ char *query = NULL;
+ int position = 0;
+
+ PyErr_Fetch(&e, &v, &tb);
+ if (e != NULL && PyErr_GivenExceptionMatches(v, PLy_exc_spi_error))
+ PLy_spi_error_data(v, &hint, &query, &position);
+ PyErr_Restore(e, v, tb);
xmsg = PLy_traceback(&xlevel);
@@ -3583,10 +3600,16 @@ PLy_elog(int elevel, const char *fmt,...)
if (fmt)
ereport(elevel,
(errmsg("PL/Python: %s", emsg.data),
- (xmsg) ? errdetail("%s", xmsg) : 0));
+ (xmsg) ? errdetail("%s", xmsg) : 0,
+ (hint) ? errhint(hint) : 0,
+ (query) ? internalerrquery(query) : 0,
+ (position) ? internalerrposition(position) : 0));
else
ereport(elevel,
- (errmsg("PL/Python: %s", xmsg)));
+ (errmsg("PL/Python: %s", xmsg),
+ (hint) ? errhint(hint) : 0,
+ (query) ? internalerrquery(query) : 0,
+ (position) ? internalerrposition(position) : 0));
}
PG_CATCH();
{
@@ -3604,6 +3627,28 @@ PLy_elog(int elevel, const char *fmt,...)
pfree(xmsg);
}
+/*
+ * Extract the error data from a SPIError
+ */
+static void
+PLy_spi_error_data(PyObject *exc, char **hint, char **query, int *position)
+{
+ PyObject *spidata = NULL;
+
+ spidata = PyObject_GetAttrString(exc, "spidata");
+ if (!spidata)
+ goto cleanup;
+
+ if (!PyArg_ParseTuple(spidata, "zzi", hint, query, position))
+ goto cleanup;
+
+cleanup:
+ PyErr_Clear();
+ /* no elog here, we simply won't report the errhint, errposition etc */
+ Py_XDECREF(spidata);
+}
+
+
static char *
PLy_traceback(int *xlevel)
{
--
1.7.2.3
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
