pgsql: Fix broken logic for reporting PL/Python function names in errco

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-committers(at)postgresql(dot)org
Subject: pgsql: Fix broken logic for reporting PL/Python function names in errco
Date: 2018-02-14 19:47:42
Message-ID: E1em31q-0008Jh-55@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

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

Branch
------
REL9_4_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/bd871863787bc09ababd7e81a189492a1cd08803

Modified Files
--------------
src/pl/plpython/expected/plpython_error.out | 23 ++++++++++
src/pl/plpython/expected/plpython_error_0.out | 23 ++++++++++
src/pl/plpython/expected/plpython_error_5.out | 23 ++++++++++
.../plpython/expected/plpython_subtransaction.out | 2 +-
src/pl/plpython/plpy_main.c | 52 +++++++++++-----------
src/pl/plpython/plpy_procedure.c | 4 +-
src/pl/plpython/sql/plpython_error.sql | 16 +++++++
7 files changed, 113 insertions(+), 30 deletions(-)

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2018-02-14 20:06:08 pgsql: Add an assertion that we don't pass NULL to snprintf("%s").
Previous Message Andres Freund 2018-02-14 18:35:08 Re: [COMMITTERS] pgsql: Rearm statement_timeout after each executed query.