Fix broken logic for reporting PL/Python function names in errcontext.

plpython_error_callback() reported the name of the function associated
with the topmost PL/Python execution context.  This was not merely
wrong if there were nested PL/Python contexts, but it risked a core
dump if the topmost one is an inline code block rather than a named
function.  That will have proname = NULL, and so we were passing a NULL
pointer to snprintf("%s").  It seems that none of the PL/Python-testing
machines in the buildfarm will dump core for that, but some platforms do,
as reported by Marina Polyakova.

Investigation finds that there actually is an existing regression test
that used to prove that the behavior was wrong, though apparently no one
had noticed that it was printing the wrong function name.  It stopped
showing the problem in 9.6 when we adjusted psql to not print CONTEXT
by default for NOTICE messages.  The problem is masked (if your platform
avoids the core dump) in error cases, because PL/Python will throw away
the originally generated error info in favor of a new traceback produced
at the outer level.

Repair by using ErrorContextCallback.arg to pass the correct context to
the error callback.  Add a regression test illustrating correct behavior.

Back-patch to all supported branches, since they're all broken this way.

Discussion: https://postgr.es/m/156b989dbc6fe7c4d3223cf51da61195@postgrespro.ru

7 files changed, 113 insertions, 30 deletions

diff --git a/src/pl/plpython/expected/plpython_error.out b/src/pl/plpython/expected/plpython_error.out
index be2ec9708a..def2fe9c75 100644
--- a/src/pl/plpython/expected/plpython_error.out
+++ b/src/pl/plpython/expected/plpython_error.out
@@ -426,3 +426,26 @@ EXCEPTION WHEN SQLSTATE 'SILLY' THEN
 	-- NOOP
 END
 $$ LANGUAGE plpgsql;
+/* test the context stack trace for nested execution levels
+ */
+CREATE FUNCTION notice_innerfunc() RETURNS int AS $$
+plpy.execute("DO LANGUAGE plpythonu $x$ plpy.notice('inside DO') $x$")
+return 1
+$$ LANGUAGE plpythonu;
+CREATE FUNCTION notice_outerfunc() RETURNS int AS $$
+plpy.execute("SELECT notice_innerfunc()")
+return 1
+$$ LANGUAGE plpythonu;
+\set SHOW_CONTEXT always
+SELECT notice_outerfunc();
+NOTICE:  inside DO
+CONTEXT:  PL/Python anonymous code block
+SQL statement "DO LANGUAGE plpythonu $x$ plpy.notice('inside DO') $x$"
+PL/Python function "notice_innerfunc"
+SQL statement "SELECT notice_innerfunc()"
+PL/Python function "notice_outerfunc"
+ notice_outerfunc 
+------------------
+                1
+(1 row)
+
diff --git a/src/pl/plpython/expected/plpython_error_0.out b/src/pl/plpython/expected/plpython_error_0.out
index 39c63c547a..bf0f661653 100644
--- a/src/pl/plpython/expected/plpython_error_0.out
+++ b/src/pl/plpython/expected/plpython_error_0.out
@@ -426,3 +426,26 @@ EXCEPTION WHEN SQLSTATE 'SILLY' THEN
 	-- NOOP
 END
 $$ LANGUAGE plpgsql;
+/* test the context stack trace for nested execution levels
+ */
+CREATE FUNCTION notice_innerfunc() RETURNS int AS $$
+plpy.execute("DO LANGUAGE plpythonu $x$ plpy.notice('inside DO') $x$")
+return 1
+$$ LANGUAGE plpythonu;
+CREATE FUNCTION notice_outerfunc() RETURNS int AS $$
+plpy.execute("SELECT notice_innerfunc()")
+return 1
+$$ LANGUAGE plpythonu;
+\set SHOW_CONTEXT always
+SELECT notice_outerfunc();
+NOTICE:  inside DO
+CONTEXT:  PL/Python anonymous code block
+SQL statement "DO LANGUAGE plpythonu $x$ plpy.notice('inside DO') $x$"
+PL/Python function "notice_innerfunc"
+SQL statement "SELECT notice_innerfunc()"
+PL/Python function "notice_outerfunc"
+ notice_outerfunc 
+------------------
+                1
+(1 row)
+
diff --git a/src/pl/plpython/expected/plpython_error_5.out b/src/pl/plpython/expected/plpython_error_5.out
index fcd944cfa2..e1d1e180a7 100644
--- a/src/pl/plpython/expected/plpython_error_5.out
+++ b/src/pl/plpython/expected/plpython_error_5.out
@@ -426,3 +426,26 @@ EXCEPTION WHEN SQLSTATE 'SILLY' THEN
 	-- NOOP
 END
 $$ LANGUAGE plpgsql;
+/* test the context stack trace for nested execution levels
+ */
+CREATE FUNCTION notice_innerfunc() RETURNS int AS $$
+plpy.execute("DO LANGUAGE plpythonu $x$ plpy.notice('inside DO') $x$")
+return 1
+$$ LANGUAGE plpythonu;
+CREATE FUNCTION notice_outerfunc() RETURNS int AS $$
+plpy.execute("SELECT notice_innerfunc()")
+return 1
+$$ LANGUAGE plpythonu;
+\set SHOW_CONTEXT always
+SELECT notice_outerfunc();
+NOTICE:  inside DO
+CONTEXT:  PL/Python anonymous code block
+SQL statement "DO LANGUAGE plpythonu $x$ plpy.notice('inside DO') $x$"
+PL/Python function "notice_innerfunc"
+SQL statement "SELECT notice_innerfunc()"
+PL/Python function "notice_outerfunc"
+ notice_outerfunc 
+------------------
+                1
+(1 row)
+
diff --git a/src/pl/plpython/expected/plpython_subtransaction.out b/src/pl/plpython/expected/plpython_subtransaction.out
index c7bf6ccd42..0a2599f288 100644
--- a/src/pl/plpython/expected/plpython_subtransaction.out
+++ b/src/pl/plpython/expected/plpython_subtransaction.out
@@ -182,7 +182,7 @@ SELECT subtransaction_deeply_nested_test();
 NOTICE:  Swallowed SyntaxError('syntax error at or near "error"',)
 CONTEXT:  PL/Python function "subtransaction_nested_test"
 SQL statement "SELECT subtransaction_nested_test('t')"
-PL/Python function "subtransaction_nested_test"
+PL/Python function "subtransaction_deeply_nested_test"
  subtransaction_deeply_nested_test 
 -----------------------------------
  ok
diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c
index 8699560306..26aa590b40 100644
--- a/src/pl/plpython/plpy_main.c
+++ b/src/pl/plpython/plpy_main.c
@@ -257,23 +257,26 @@ plpython_call_handler(PG_FUNCTION_ARGS)
 	/*
 	 * Push execution context onto stack.  It is important that this get
 	 * popped again, so avoid putting anything that could throw error between
-	 * here and the PG_TRY.  (plpython_error_callback expects the stack entry
-	 * to be there, so we have to make the context first.)
+	 * here and the PG_TRY.
 	 */
 	exec_ctx = PLy_push_execution_context();
 
-	/*
-	 * Setup error traceback support for ereport()
-	 */
-	plerrcontext.callback = plpython_error_callback;
-	plerrcontext.previous = error_context_stack;
-	error_context_stack = &plerrcontext;
-
 	PG_TRY();
 	{
 		Oid			funcoid = fcinfo->flinfo->fn_oid;
 		PLyProcedure *proc;
 
+		/*
+		 * Setup error traceback support for ereport().  Note that the PG_TRY
+		 * structure pops this for us again at exit, so we needn't do that
+		 * explicitly, nor do we risk the callback getting called after we've
+		 * destroyed the exec_ctx.
+		 */
+		plerrcontext.callback = plpython_error_callback;
+		plerrcontext.arg = exec_ctx;
+		plerrcontext.previous = error_context_stack;
+		error_context_stack = &plerrcontext;
+
 		if (CALLED_AS_TRIGGER(fcinfo))
 		{
 			Relation	tgrel = ((TriggerData *) fcinfo->context)->tg_relation;
@@ -299,9 +302,7 @@ plpython_call_handler(PG_FUNCTION_ARGS)
 	}
 	PG_END_TRY();
 
-	/* Pop the error context stack */
-	error_context_stack = plerrcontext.previous;
-	/* ... and then the execution context */
+	/* Destroy the execution context */
 	PLy_pop_execution_context();
 
 	return retval;
@@ -344,21 +345,22 @@ plpython_inline_handler(PG_FUNCTION_ARGS)
 	/*
 	 * Push execution context onto stack.  It is important that this get
 	 * popped again, so avoid putting anything that could throw error between
-	 * here and the PG_TRY.  (plpython_inline_error_callback doesn't currently
-	 * need the stack entry, but for consistency with plpython_call_handler we
-	 * do it in this order.)
+	 * here and the PG_TRY.
 	 */
 	exec_ctx = PLy_push_execution_context();
 
-	/*
-	 * Setup error traceback support for ereport()
-	 */
-	plerrcontext.callback = plpython_inline_error_callback;
-	plerrcontext.previous = error_context_stack;
-	error_context_stack = &plerrcontext;
-
 	PG_TRY();
 	{
+		/*
+		 * Setup error traceback support for ereport().
+		 * plpython_inline_error_callback doesn't currently need exec_ctx, but
+		 * for consistency with plpython_call_handler we do it the same way.
+		 */
+		plerrcontext.callback = plpython_inline_error_callback;
+		plerrcontext.arg = exec_ctx;
+		plerrcontext.previous = error_context_stack;
+		error_context_stack = &plerrcontext;
+
 		PLy_procedure_compile(&proc, codeblock->source_text);
 		exec_ctx->curr_proc = &proc;
 		PLy_exec_function(&fake_fcinfo, &proc);
@@ -372,9 +374,7 @@ plpython_inline_handler(PG_FUNCTION_ARGS)
 	}
 	PG_END_TRY();
 
-	/* Pop the error context stack */
-	error_context_stack = plerrcontext.previous;
-	/* ... and then the execution context */
+	/* Destroy the execution context */
 	PLy_pop_execution_context();
 
 	/* Now clean up the transient procedure we made */
@@ -402,7 +402,7 @@ PLy_procedure_is_trigger(Form_pg_proc procStruct)
 static void
 plpython_error_callback(void *arg)
 {
-	PLyExecutionContext *exec_ctx = PLy_current_execution_context();
+	PLyExecutionContext *exec_ctx = (PLyExecutionContext *) arg;
 
 	if (exec_ctx->curr_proc)
 		errcontext("PL/Python function \"%s\"",
diff --git a/src/pl/plpython/plpy_procedure.c b/src/pl/plpython/plpy_procedure.c
index 9e62f46370..27b79ee962 100644
--- a/src/pl/plpython/plpy_procedure.c
+++ b/src/pl/plpython/plpy_procedure.c
@@ -45,9 +45,7 @@ init_procedure_caches(void)
 }
 
 /*
- * Get the name of the last procedure called by the backend (the
- * innermost, if a plpython procedure call calls the backend and the
- * backend calls another plpython procedure).
+ * PLy_procedure_name: get the name of the specified procedure.
  *
  * NB: this returns the SQL name, not the internal Python procedure name
  */
diff --git a/src/pl/plpython/sql/plpython_error.sql b/src/pl/plpython/sql/plpython_error.sql
index d0df7e607d..d712eb1078 100644
--- a/src/pl/plpython/sql/plpython_error.sql
+++ b/src/pl/plpython/sql/plpython_error.sql
@@ -328,3 +328,19 @@ EXCEPTION WHEN SQLSTATE 'SILLY' THEN
 	-- NOOP
 END
 $$ LANGUAGE plpgsql;
+
+/* test the context stack trace for nested execution levels
+ */
+CREATE FUNCTION notice_innerfunc() RETURNS int AS $$
+plpy.execute("DO LANGUAGE plpythonu $x$ plpy.notice('inside DO') $x$")
+return 1
+$$ LANGUAGE plpythonu;
+
+CREATE FUNCTION notice_outerfunc() RETURNS int AS $$
+plpy.execute("SELECT notice_innerfunc()")
+return 1
+$$ LANGUAGE plpythonu;
+
+\set SHOW_CONTEXT always
+
+SELECT notice_outerfunc();