Received: from magus.postgresql.org (magus.postgresql.org [87.238.57.229]) by mail.postgresql.org (Postfix) with ESMTP id B497C2EBA7C for ; Thu, 24 Nov 2011 04:07:16 -0400 (AST) Received: from wulczer.org ([109.74.195.157]) by magus.postgresql.org with esmtp (Exim 4.72) (envelope-from ) id 1RTUKx-0005zl-KU for pgsql-hackers@postgresql.org; Thu, 24 Nov 2011 08:07:16 +0000 Received: from [85.59.228.11] (helo=[10.0.0.101]) by wulczer.org with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.72) (envelope-from ) id 1RTUKk-0004sR-BM; Thu, 24 Nov 2011 08:07:02 +0000 Message-ID: <4ECDFB26.1090203@wulczer.org> Date: Thu, 24 Nov 2011 09:07:02 +0100 From: =?UTF-8?B?SmFuIFVyYmHFhHNraQ==?= User-Agent: Mozilla/5.0 (X11; Linux i686; rv:8.0) Gecko/20111105 Thunderbird/8.0 MIME-Version: 1.0 To: Mika Eloranta CC: pgsql-hackers@postgresql.org Subject: Re: PL/Python SQL error code pass-through References: <0174B71F-7940-4819-BDC8-95EC8C9BA833@ohmu.fi> In-Reply-To: <0174B71F-7940-4819-BDC8-95EC8C9BA833@ohmu.fi> Content-Type: multipart/mixed; boundary="------------090508020002000804030205" X-Archive-Number: 201111/1377 X-Sequence-Number: 198862 This is a multi-part message in MIME format. --------------090508020002000804030205 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 23/11/11 17:24, Mika Eloranta wrote: > Hi all, > > [PL/Python in 9.1 does not preserve SQLSTATE of errors] Oops, you're right, it's a regression from 9.0 behaviour. The fix looks good to me, I changed one place to indent with tabs instead of spaces and added a regression test. I think this should be backpatched to 9.1, no? Thanks for the report and the patch! Cheers, Jan --------------090508020002000804030205 Content-Type: text/x-diff; name="plpython-error-passthrough-w-test.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="plpython-error-passthrough-w-test.patch" diff --git a/src/pl/plpython/expected/plpython_error.out b/src/pl/plpython/expected/plpython_error.out index dbf19fd..bab07fb 100644 *** a/src/pl/plpython/expected/plpython_error.out --- b/src/pl/plpython/expected/plpython_error.out *************** CONTEXT: PL/Python function "specific_e *** 351,356 **** --- 351,378 ---- (1 row) + /* SPI errors in PL/Python functions should preserve the SQLSTATE value + */ + CREATE FUNCTION python_unique_violation() RETURNS void AS $$ + plpy.execute("insert into specific values (1)") + plpy.execute("insert into specific values (1)") + $$ LANGUAGE plpythonu; + CREATE FUNCTION catch_python_unique_violation() RETURNS text AS $$ + begin + begin + perform python_unique_violation(); + exception when unique_violation then + return 'ok'; + end; + return 'not reached'; + end; + $$ language plpgsql; + SELECT catch_python_unique_violation(); + catch_python_unique_violation + ------------------------------- + ok + (1 row) + /* manually starting subtransactions - a bad idea */ CREATE FUNCTION manual_subxact() RETURNS void AS $$ diff --git a/src/pl/plpython/expected/plpython_error_0.out b/src/pl/plpython/expected/plpython_error_0.out index b2194ff..6cb2ed0 100644 *** a/src/pl/plpython/expected/plpython_error_0.out --- b/src/pl/plpython/expected/plpython_error_0.out *************** CONTEXT: PL/Python function "specific_e *** 351,356 **** --- 351,378 ---- (1 row) + /* SPI errors in PL/Python functions should preserve the SQLSTATE value + */ + CREATE FUNCTION python_unique_violation() RETURNS void AS $$ + plpy.execute("insert into specific values (1)") + plpy.execute("insert into specific values (1)") + $$ LANGUAGE plpythonu; + CREATE FUNCTION catch_python_unique_violation() RETURNS text AS $$ + begin + begin + perform python_unique_violation(); + exception when unique_violation then + return 'ok'; + end; + return 'not reached'; + end; + $$ language plpgsql; + SELECT catch_python_unique_violation(); + catch_python_unique_violation + ------------------------------- + ok + (1 row) + /* manually starting subtransactions - a bad idea */ CREATE FUNCTION manual_subxact() RETURNS void AS $$ diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c index 93e8043..afd5dfc 100644 *** a/src/pl/plpython/plpython.c --- b/src/pl/plpython/plpython.c *************** static char *PLy_procedure_name(PLyProce *** 383,389 **** static void PLy_elog(int, const char *,...) __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 3))); ! static void PLy_get_spi_error_data(PyObject *exc, char **detail, char **hint, char **query, int *position); static void PLy_traceback(char **, char **, int *); static void *PLy_malloc(size_t); --- 383,389 ---- static void PLy_elog(int, const char *,...) __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 3))); ! static void PLy_get_spi_error_data(PyObject *exc, int *sqlerrcode, char **detail, char **hint, char **query, int *position); static void PLy_traceback(char **, char **, int *); static void *PLy_malloc(size_t); *************** PLy_spi_exception_set(PyObject *excclass *** 4441,4447 **** if (!spierror) goto failure; ! spidata = Py_BuildValue("(zzzi)", edata->detail, edata->hint, edata->internalquery, edata->internalpos); if (!spidata) goto failure; --- 4441,4447 ---- if (!spierror) goto failure; ! spidata = Py_BuildValue("(izzzi)", edata->sqlerrcode, edata->detail, edata->hint, edata->internalquery, edata->internalpos); if (!spidata) goto failure; *************** PLy_elog(int elevel, const char *fmt,... *** 4481,4486 **** --- 4481,4487 ---- *val, *tb; const char *primary = NULL; + int sqlerrcode = 0; char *detail = NULL; char *hint = NULL; char *query = NULL; *************** PLy_elog(int elevel, const char *fmt,... *** 4490,4496 **** if (exc != NULL) { if (PyErr_GivenExceptionMatches(val, PLy_exc_spi_error)) ! PLy_get_spi_error_data(val, &detail, &hint, &query, &position); else if (PyErr_GivenExceptionMatches(val, PLy_exc_fatal)) elevel = FATAL; } --- 4491,4497 ---- if (exc != NULL) { if (PyErr_GivenExceptionMatches(val, PLy_exc_spi_error)) ! PLy_get_spi_error_data(val, &sqlerrcode, &detail, &hint, &query, &position); else if (PyErr_GivenExceptionMatches(val, PLy_exc_fatal)) elevel = FATAL; } *************** PLy_elog(int elevel, const char *fmt,... *** 4531,4537 **** PG_TRY(); { ereport(elevel, ! (errmsg_internal("%s", primary ? primary : "no exception data"), (detail) ? errdetail_internal("%s", detail) : 0, (tb_depth > 0 && tbmsg) ? errcontext("%s", tbmsg) : 0, (hint) ? errhint("%s", hint) : 0, --- 4532,4539 ---- PG_TRY(); { ereport(elevel, ! (errcode(sqlerrcode ? sqlerrcode : ERRCODE_INTERNAL_ERROR), ! errmsg_internal("%s", primary ? primary : "no exception data"), (detail) ? errdetail_internal("%s", detail) : 0, (tb_depth > 0 && tbmsg) ? errcontext("%s", tbmsg) : 0, (hint) ? errhint("%s", hint) : 0, *************** PLy_elog(int elevel, const char *fmt,... *** 4562,4568 **** * Extract the error data from a SPIError */ static void ! PLy_get_spi_error_data(PyObject *exc, char **detail, char **hint, char **query, int *position) { PyObject *spidata = NULL; --- 4564,4570 ---- * Extract the error data from a SPIError */ static void ! PLy_get_spi_error_data(PyObject *exc, int* sqlerrcode, char **detail, char **hint, char **query, int *position) { PyObject *spidata = NULL; *************** PLy_get_spi_error_data(PyObject *exc, ch *** 4570,4576 **** if (!spidata) goto cleanup; ! if (!PyArg_ParseTuple(spidata, "zzzi", detail, hint, query, position)) goto cleanup; cleanup: --- 4572,4578 ---- if (!spidata) goto cleanup; ! if (!PyArg_ParseTuple(spidata, "izzzi", sqlerrcode, detail, hint, query, position)) goto cleanup; cleanup: diff --git a/src/pl/plpython/sql/plpython_error.sql b/src/pl/plpython/sql/plpython_error.sql index 4add6aa..502bbec 100644 *** a/src/pl/plpython/sql/plpython_error.sql --- b/src/pl/plpython/sql/plpython_error.sql *************** SELECT specific_exception(2); *** 257,262 **** --- 257,282 ---- SELECT specific_exception(NULL); SELECT specific_exception(2); + /* SPI errors in PL/Python functions should preserve the SQLSTATE value + */ + CREATE FUNCTION python_unique_violation() RETURNS void AS $$ + plpy.execute("insert into specific values (1)") + plpy.execute("insert into specific values (1)") + $$ LANGUAGE plpythonu; + + CREATE FUNCTION catch_python_unique_violation() RETURNS text AS $$ + begin + begin + perform python_unique_violation(); + exception when unique_violation then + return 'ok'; + end; + return 'not reached'; + end; + $$ language plpgsql; + + SELECT catch_python_unique_violation(); + /* manually starting subtransactions - a bad idea */ CREATE FUNCTION manual_subxact() RETURNS void AS $$ --------------090508020002000804030205--