>From f8348e20bfe0e7b9e3df076947261b073c1053a7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Urba=C5=84ski?= <wulczer@wulczer.org>
Date: Sat, 1 Jan 2011 22:48:09 +0100
Subject: [PATCH 02/16] 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 |   22 ++++----
 src/pl/plpython/expected/plpython_test.out  |    2 +-
 src/pl/plpython/plpython.c                  |   78 +++++++--------------------
 3 files changed, 30 insertions(+), 72 deletions(-)

diff --git a/src/pl/plpython/expected/plpython_error.out b/src/pl/plpython/expected/plpython_error.out
index 1f24c13..57488b0 100644
--- a/src/pl/plpython/expected/plpython_error.out
+++ b/src/pl/plpython/expected/plpython_error.out
@@ -10,10 +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"
-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"
 /* check the handling of uncaught python exceptions
  */
@@ -34,11 +31,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
-LINE 1: SELECT test5('foo')
-               ^
-HINT:  No function matches the given name and argument types. You might need to add explicit type casts.
-QUERY:  SELECT test5('foo')
+ERROR:  PL/Python: plpy.SPIError: function test5(unknown) does not exist
 CONTEXT:  PL/Python function "exception_index_invalid_nested"
 /* a typo
  */
@@ -56,7 +49,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 +72,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 +99,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 9acaba2..64a8a4a 100644
--- a/src/pl/plpython/plpython.c
+++ b/src/pl/plpython/plpython.c
@@ -361,18 +361,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;
@@ -592,7 +580,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
@@ -1009,7 +996,6 @@ PLy_function_handler(FunctionCallInfo fcinfo, PLyProcedure *proc)
 				 */
 				PLy_function_delete_args(proc);
 			Assert(plrv != NULL);
-			Assert(!PLy_error_in_progress);
 		}
 
 		/*
@@ -1184,19 +1170,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;
 }
@@ -2798,12 +2775,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))
 	{
@@ -2914,15 +2885,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_exception_set(PLy_exc_spi_error, edata->message);
 		return NULL;
 	}
 	PG_END_TRY();
@@ -2941,13 +2914,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);
 
@@ -3052,9 +3018,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();
 
 		/*
@@ -3074,6 +3041,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_exception_set(PLy_exc_spi_error, edata->message);
 		return NULL;
 	}
 	PG_END_TRY();
@@ -3088,13 +3056,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);
 }
@@ -3113,13 +3074,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_exception_set(PLy_exc_spi_error, edata->message);
 		return NULL;
 	}
 	PG_END_TRY();
@@ -3180,8 +3144,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");
@@ -3421,23 +3383,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();
-- 
1.7.2.3

