8000 Fix broken logic for reporting PL/Python function names in errcontext. · postgres/postgres@0a2381d · GitHub
[go: up one dir, main page]
Skip to content

Commit 0a2381d

Browse files
tglsfdctglsfdc
committed
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
1 parent 4b7f418 commit 0a2381d

File tree

6 files changed

+112
-29
lines changed

6 files changed

+112
-29
lines changed

‎src/pl/plpython/expected/plpython_error.out

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,3 +422,26 @@ EXCEPTION WHEN SQLSTATE 'SILLY' THEN
422422
-- NOOP
423423
END
424424
$$ LANGUAGE plpgsql;
425+
/* test the context stack trace for nested execution levels
426+
*/
427+
CREATE FUNCTION notice_innerfunc() RETURNS int AS $$
428+
plpy.execute("DO LANGUAGE plpythonu $x$ plpy.notice('inside DO') $x$")
429+
return 1
430+
$$ LANGUAGE plpythonu;
431+
CREATE FUNCTION notice_outerfunc() RETURNS int AS $$
432+
plpy.execute("SELECT notice_innerfunc()")
433+
return 1
434+
$$ LANGUAGE plpythonu;
435+
\set SHOW_CONTEXT always
436+
SELECT notice_outerfunc();
437+
NOTICE: inside DO
438+
CONTEXT: PL/Python anonymous code block
439+
SQL statement "DO LANGUAGE plpythonu $x$ plpy.notice('inside DO') $x$"
440+
PL/Python function "notice_innerfunc"
441+
SQL statement "SELECT notice_innerfunc()"
442+
PL/Python function "notice_outerfunc"
443+
notice_outerfunc
444+
------------------
445+
1
446+
(1 row)
447+

‎src/pl/plpython/expected/plpython_error_0.out

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,3 +422,26 @@ EXCEPTION WHEN SQLSTATE 'SILLY' THEN
422422
-- NOOP
423423
END
424424
$$ LANGUAGE plpgsql;
425+
/* test the context stack trace for nested execution levels
426+
*/
427+
CREATE FUNCTION notice_innerfunc() RETURNS int AS $$
428+
plpy.execute("DO LANGUAGE plpythonu $x$ plpy.notice('inside DO') $x$")
429+
return 1
430+
$$ LANGUAGE plpythonu;
431+
CREATE FUNCTION notice_outerfunc() RETURNS int AS $$
432+
plpy.execute("SELECT notice_innerfunc()")
433+
return 1
434+
$$ LANGUAGE plpythonu;
435+
\set SHOW_CONTEXT always
436+
SELECT notice_outerfunc();
437+
NOTICE: inside DO
438+
CONTEXT: PL/Python anonymous code block
439+
SQL statement "DO LANGUAGE plpythonu $x$ plpy.notice('inside DO') $x$"
440+
PL/Python function "notice_innerfunc"
441+
SQL statement "SELECT notice_innerfunc()"
442+
PL/Python function "notice_outerfunc"
443+
notice_outerfunc
444+
------------------
445+
1
446+
(1 row)
447+

‎src/pl/plpython/expected/plpython_error_5.out

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,3 +422,26 @@ EXCEPTION WHEN SQLSTATE 'SILLY' THEN
422422
-- NOOP
423423
END
424424
$$ LANGUAGE plpgsql;
425+
/* test the context stack trace for nested execution levels
426+
*/
427+
CREATE FUNCTION notice_innerfunc() RETURNS int AS $$
428+
plpy.execute("DO LANGUAGE plpythonu $x$ plpy.notice('inside DO') $x$")
429+
return 1
430+
$$ LANGUAGE plpythonu;
431+
CREATE FUN 8000 CTION notice_outerfunc() RETURNS int AS $$
432+
plpy.execute("SELECT notice_innerfunc()")
433+
return 1
434+
$$ LANGUAGE plpythonu;
435+
\set SHOW_CONTEXT always
436+
SELECT notice_outerfunc();
437+
NOTICE: inside DO
438+
CONTEXT: PL/Python anonymous code block
439+
SQL statement "DO LANGUAGE plpythonu $x$ plpy.notice('inside DO') $x$"
440+
PL/Python function "notice_innerfunc"
441+
SQL statement "SELECT notice_innerfunc()"
442+
PL/Python function "notice_outerfunc"
443+
notice_outerfunc
444+
------------------
445+
1
446+
(1 row)
447+

‎src/pl/plpython/plpy_main.c

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -232,23 +232,26 @@ plpython_call_handler(PG_FUNCTION_ARGS)
232232
/*
233233
* Push execution context onto stack. It is important that this get
234234
* popped again, so avoid putting anything that could throw error between
235-
* here and the PG_TRY. (plpython_error_callback expects the stack entry
236-
* to be there, so we have to make the context first.)
235+
* here and the PG_TRY.
237236
*/
238237
exec_ctx = PLy_push_execution_context();
239238

240-
/*
241-
* Setup error traceback support for ereport()
242-
*/
243-
plerrcontext.callback = plpython_error_callback;
244-
plerrcontext.previous = error_context_stack;
245-
error_context_stack = &plerrcontext;
246-
247239
PG_TRY();
248240
{
249241
Oid funcoid = fcinfo->flinfo->fn_oid;
250242
PLyProcedure *proc;
251243

244+
/*
245+
* Setup error traceback support for ereport(). Note that the PG_TRY
246+
* structure pops this for us again at exit, so we needn't do that
247+
* explicitly, nor do we risk the callback getting called after we've
248+
* destroyed the exec_ctx.
249+
*/
250+
plerrcontext.callback = plpython_error_callback;
251+
plerrcontext.arg = exec_ctx;
252+
plerrcontext.previous = error_context_stack;
253+
error_context_stack = &plerrcontext;
254+
252255
if (CALLED_AS_TRIGGER(fcinfo))
253256
{
254257
Relation tgrel = ((TriggerData *) fcinfo->context)->tg_relation;
@@ -274,9 +277,7 @@ plpython_call_handler(PG_FUNCTION_ARGS)
274277
}
275278
PG_END_TRY();
276279

277-
/* Pop the error context stack */
278-
error_context_stack = plerrcontext.previous;
279-
/* ... and then the execution context */
280+
/* Destroy the execution context */
280281
PLy_pop_execution_context();
281282

282283
return retval;
@@ -323,21 +324,22 @@ plpython_inline_handler(PG_FUNCTION_ARGS)
323324
/*
324325
* Push execution context onto stack. It is important that this get
325326
* popped again, so avoid putting anything that could throw error between
326-
* here and the PG_TRY. (plpython_inline_error_callback doesn't currently
327-
* need the stack entry, but for consistency with plpython_call_handler we
328-
* do it in this order.)
327+
* here and the PG_TRY.
329328
*/
330329
exec_ctx = PLy_push_execution_context();
331330

332-
/*
333-
* Setup error traceback support for ereport()
334-
*/
335-
plerrcontext.callback = plpython_inline_error_callback;
336-
plerrcontext.previous = error_context_stack;
337-
error_context_stack = &plerrcontext;
338-
339331
PG_TRY();
340332
{
333+
/*
334+
* Setup error traceback support for ereport().
335+
* plpython_inline_error_callback doesn't currently need exec_ctx, but
336+
* for consistency with plpython_call_handler we do it the same way.
337+
*/
338+
plerrcontext.callback = plpython_inline_error_callback;
339+
plerrcontext.arg = exec_ctx;
340+
plerrcontext.previous = error_context_stack;
341+
error_context_stack = &plerrcontext;
342+
341343
PLy_procedure_compile(&proc, codeblock->source_text);
342344
exec_ctx->curr_proc = &proc;
343345
PLy_exec_function(&fake_fcinfo, &proc);
@@ -351,9 +353,7 @@ plpython_inline_handler(PG_FUNCTION_ARGS)
351353
}
352354
PG_END_TRY();
353355

354-
/* Pop the error context stack */
355-
error_context_stack = plerrcontext.previous;
356-
/* ... and then the execution context */
356+
/* Destroy the execution context */
357357
PLy_pop_execution_context();
358358

359359
/* Now clean up the transient procedure we made */
@@ -381,7 +381,7 @@ PLy_procedure_is_trigger(Form_pg_proc procStruct)
381381
static void
382382
plpython_error_callback(void *arg)
383383
{
384-
PLyExecutionContext *exec_ctx = PLy_current_execution_context();
384+
PLyExecutionContext *exec_ctx = (PLyExecutionContext *) arg;
385385

386386
if (exec_ctx->curr_proc)
387387
errcontext("PL/Python function \"%s\"",

‎src/pl/plpython/plpy_procedure.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,7 @@ init_procedure_caches(void)
4747
}
4848

4949
/*
50-
* Get the name of the last procedure called by the backend (the
51-
* innermost, if a plpython procedure call calls the backend and the
52-
* backend calls another plpython procedure).
50+
* PLy_procedure_name: get the name of the specified procedure.
5351
*
5452
* NB: this returns the SQL name, not the internal Python procedure name
5553
*/

‎src/pl/plpython/sql/plpython_error.sql

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,3 +328,19 @@ EXCEPTION WHEN SQLSTATE 'SILLY' THEN
328328
-- NOOP
329329
END
330330
$$ LANGUAGE plpgsql;
331+
332+
/* test the context stack trace for nested execution levels
333+
*/
334+
CREATE FUNCTION notice_innerfunc() RETURNS int AS $$
335+
plpy.execute("DO LANGUAGE plpythonu $x$ plpy.notice('inside DO') $x$")
336+
return 1
337+
$$ LANGUAGE plpythonu;
338+
339+
CREATE FUNCTION notice_outerfunc() RETURNS int AS $$
340+
plpy.execute("SELECT notice_innerfunc()")
341+
return 1
342+
$$ LANGUAGE plpythonu;
343+
344+
\set SHOW_CONTEXT always
345+
346+
SELECT notice_outerfunc();

0 commit comments

Comments
 (0)
0