Hi 2016-03-01 22:23 GMT+01:00 Jim Nasby <jim.na...@bluetreble.com>:
> On 2/29/16 10:01 PM, Tom Lane wrote: > >> Jim Nasby <jim.na...@bluetreble.com> writes: >> >>> On 2/28/16 5:50 PM, Jim Nasby wrote: >>> >>>> Per discussion in [1], this patch improves error reporting in pltcl. >>>> >>> >> I forgot to mention that this work is sponsored by Flight Aware >>> (http://flightaware.com). >>> >> >> Huh ... I use that site. There's PG and pltcl code behind it? >> Cool! >> > > I didn't study this patch deeper yet. Just I am sending rebased code I found one issue. The context + ERROR: relation "foo" does not exist + CONTEXT: relation "foo" does not exist + while executing + "spi_exec "select * from foo;"" + ("eval" body line 1) + invoked from within + "eval $1" + (procedure "__PLTcl_proc_16461" line 3) + invoked from within + "__PLTcl_proc_16461 {spi_exec "select * from foo;"}" + in PL/Tcl function "tcl_eval" is changed in any call - when I did "make installcheck" *** 567,575 **** ("eval" body line 1) invoked from within "eval $1" ! (procedure "__PLTcl_proc_16461" line 3) invoked from within ! "__PLTcl_proc_16461 {spi_exec "select * from foo;"}" in PL/Tcl function "tcl_eval" select pg_temp.tcl_eval($$ set list [lindex $::errorCode 0]; --- 567,575 ---- ("eval" body line 1) invoked from within "eval $1" ! (procedure "__PLTcl_proc_16841" line 3) <<<<<==================== _PLTcl_proc_XXXX invoked from within ! "__PLTcl_proc_16841 {spi_exec "select * from foo;"}" in PL/Tcl function "tcl_eval" select pg_temp.tcl_eval($$ set list [lindex $::errorCode 0]; I am not able to see, if this information is interesting or not. We can hide context, but I have not a idea, if it is ok or not. Regards Pavel
diff --git a/doc/src/sgml/pltcl.sgml b/doc/src/sgml/pltcl.sgml new file mode 100644 index d2175d5..d5c576d *** a/doc/src/sgml/pltcl.sgml --- b/doc/src/sgml/pltcl.sgml *************** CREATE EVENT TRIGGER tcl_a_snitch ON ddl *** 775,780 **** --- 775,901 ---- </para> </sect1> + <sect1 id="pltcl-error-handling"> + <title>Error Handling in PL/Tcl</title> + + <indexterm> + <primary>error handling</primary> + <secondary>in PL/Tcl</secondary> + </indexterm> + + <para> + All Tcl errors that are allowed to propagate back to the top level of the + interpreter, that is, errors not caught within the stored procedure + using the Tcl <function>catch</function> command will raise a database + error. + </para> + <para> + Tcl code within or called from the stored procedure can choose to + raise a database error by invoking the <function>elog</function> + command provided by PL/Tcl or by generating an error using the Tcl + <function>error</function> command and not catching it with Tcl's + <function>catch</function> command. + </para> + <para> + Database errors that occur from the PL/Tcl stored procedure's + use of <function>spi_exec</function>, <function>spi_prepare</function>, + and <function>spi_execp</function> are also catchable by Tcl's + <function>catch</function> command. + </para> + <para> + Tcl provides an <varname>errorCode</varname> variable that can + represent additional information about the error in a form that + is easy for programs to interpret. The contents are in Tcl list + format and the first word identifies the subsystem or + library responsible for the error and beyond that the contents are left + to the individual code or library. For example if Tcl's + <function>open</function> command is asked to open a file that doesn't + exist, <varname>errorCode</varname> + might contain <literal>POSIX ENOENT {no such file or directory}</literal> + where the third element may vary by locale but the first and second + will not. + </para> + <para> + When <function>spi_exec</function>, <function>spi_prepare</function> + or <function>spi_execp</function> cause a database error to be raised, + that database eror propagates back to Tcl as a Tcl error. + In this case <varname>errorCode</varname> is set to a list + where the first element is <literal>POSTGRES</literal> followed by a + copious decoding of the Postgres error structure. Since fields in the + structure may or may not be present depending on the nature of the + error, how the function was invoked, etc, PL/Tcl has adopted the + convention that subsequent elements of the <varname>errorCode</varname> + list are key-value pairs where the first value is the name of the + field and the second is its value. + </para> + <para> + Fields that may be present include <varname>message</varname>, + <varname>detail</varname>, <varname>detail_log</varname>, + <varname>hint</varname>, <varname>domain</varname>, + <varname>context_domain</varname>, <varname>context</varname>, + <varname>schema</varname>, <varname>table</varname>, + <varname>column</varname>, <varname>datatype</varname>, + <varname>constraint</varname>, <varname>cursor_position</varname>, + <varname>internalquery</varname>, <varname>internal_position</varname>, + <varname>filename</varname>, <varname>lineno</varname> and + <varname>funcname</varname>. + </para> + <para> + You might find it useful to load the results into an array. Code + for doing that might look like + <programlisting> + if {[lindex $errorCode 0] == "POSTGRES"} { + array set errorRow [lrange $errorCode 1 end] + } + </programlisting> + </para> + <para> + In the example below we cause an error by attempting to + <command>SELECT</> from a table that doesn't exist. + <screen> + select tcl_eval('spi_exec "select * from foo;"'); + </screen> + <screen> + <computeroutput> + ERROR: relation "foo" does not exist + </computeroutput> + </screen> + </para> + <para> + Now we examine the error code. (The double-colons explicitly + reference <varname>errorCode</varname> as a global variable.) + <screen> + select tcl_eval('join $::errorCode "\n"'); + </screen> + <screen> + <computeroutput> + tcl_eval + ------------------------------- + POSTGRES + + message + + relation "foo" does not exist+ + domain + + postgres-9.6 + + context_domain + + postgres-9.6 + + cursorpos + + 0 + + internalquery + + select * from foo; + + internalpos + + 15 + + filename + + parse_relation.c + + lineno + + 1159 + + funcname + + parserOpenTable + (1 row) + </computeroutput> + </screen> + </para> + </sect1> + <sect1 id="pltcl-unknown"> <title>Modules and the <function>unknown</> Command</title> <para> diff --git a/src/pl/tcl/expected/pltcl_setup.out b/src/pl/tcl/expected/pltcl_setup.out new file mode 100644 index e11718c..cac7597 *** a/src/pl/tcl/expected/pltcl_setup.out --- b/src/pl/tcl/expected/pltcl_setup.out *************** NOTICE: tclsnitch: ddl_command_start DR *** 555,557 **** --- 555,598 ---- NOTICE: tclsnitch: ddl_command_end DROP TABLE drop event trigger tcl_a_snitch; drop event trigger tcl_b_snitch; + -- test error handling + CREATE OR REPLACE FUNCTION pg_temp.tcl_eval (varchar) RETURNS varchar AS $$ + eval $1 + $$ LANGUAGE pltcl; + select pg_temp.tcl_eval('spi_exec "select * from foo;"'); + ERROR: relation "foo" does not exist + CONTEXT: relation "foo" does not exist + while executing + "spi_exec "select * from foo;"" + ("eval" body line 1) + invoked from within + "eval $1" + (procedure "__PLTcl_proc_16461" line 3) + invoked from within + "__PLTcl_proc_16461 {spi_exec "select * from foo;"}" + in PL/Tcl function "tcl_eval" + select pg_temp.tcl_eval($$ + set list [lindex $::errorCode 0]; + foreach "key value" [lrange $::errorCode 1 end] { + if {$key == "domain" || $key == "context_domain" || $key == "lineno"} continue; + lappend list $key $value + }; + return [join $list "\n"] + $$); + tcl_eval + ------------------------------- + POSTGRES + + message + + relation "foo" does not exist+ + cursor_position + + 0 + + internalquery + + select * from foo; + + internal_position + + 15 + + filename + + parse_relation.c + + funcname + + parserOpenTable + (1 row) + diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c new file mode 100644 index 105b618..7e064d9 *** a/src/pl/tcl/pltcl.c --- b/src/pl/tcl/pltcl.c *************** static int pltcl_SPI_lastoid(ClientData *** 237,242 **** --- 237,243 ---- static void pltcl_set_tuple_values(Tcl_Interp *interp, const char *arrayname, int tupno, HeapTuple tuple, TupleDesc tupdesc); static Tcl_Obj *pltcl_build_tuple_argument(HeapTuple tuple, TupleDesc tupdesc); + static void pltcl_construct_errorCode(Tcl_Interp *interp, ErrorData *edata); /* *************** compile_pltcl_function(Oid fn_oid, Oid t *** 1576,1581 **** --- 1577,1672 ---- return prodesc; } + /********************************************************************** + * pltcl_construct_errorCode() - construct a Tcl errorCode + * list with detailed information from the PostgreSQL server + **********************************************************************/ + static void + pltcl_construct_errorCode(Tcl_Interp *interp, ErrorData *edata) + { + Tcl_Obj *obj = Tcl_NewObj(); + + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("POSTGRES", -1)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("message", -1)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj(edata->message, -1)); + + if (edata->detail) + { + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("detail", -1)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj(edata->detail, -1)); + } + if (edata->detail_log) + { + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("detail_log", -1)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj(edata->detail_log, -1)); + } + if (edata->hint) + { + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("hint", -1)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj(edata->hint, -1)); + } + if (edata->domain) + { + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("domain", -1)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj(edata->domain, -1)); + } + if (edata->context_domain) + { + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("context_domain", -1)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj(edata->context_domain, -1)); + } + if (edata->context) + { + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("context", -1)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj(edata->context, -1)); + } + if (edata->schema_name) + { + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("schema", -1)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj(edata->schema_name, -1)); + } + if (edata->table_name) + { + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("table", -1)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj(edata->table_name, -1)); + } + if (edata->column_name) + { + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("column", -1)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj(edata->column_name, -1)); + } + if (edata->datatype_name) + { + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("datatype", -1)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj(edata->datatype_name, -1)); + } + if (edata->constraint_name) + { + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("constraint", -1)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj(edata->constraint_name, -1)); + } + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("cursor_position", -1)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewIntObj(edata->cursorpos)); + if (edata->internalquery) + { + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("internalquery", -1)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj(edata->internalquery, -1)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("internal_position", -1)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewIntObj(edata->internalpos)); + } + if (edata->filename) + { + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("filename", -1)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj(edata->filename, -1)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("lineno", -1)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewIntObj(edata->lineno)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("funcname", -1)); + Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj(edata->funcname, -1)); + } + + Tcl_SetObjErrorCode(interp, obj); + } + /********************************************************************** * pltcl_elog() - elog() support for PLTcl *************** pltcl_elog(ClientData cdata, Tcl_Interp *** 1652,1657 **** --- 1743,1749 ---- UTF_BEGIN; Tcl_SetObjResult(interp, Tcl_NewStringObj(UTF_E2U(edata->message), -1)); UTF_END; + pltcl_construct_errorCode(interp, edata); FreeErrorData(edata); return TCL_ERROR; *************** pltcl_subtrans_abort(Tcl_Interp *interp, *** 1884,1889 **** --- 1976,1982 ---- UTF_BEGIN; Tcl_SetResult(interp, UTF_E2U(edata->message), TCL_VOLATILE); UTF_END; + pltcl_construct_errorCode(interp, edata); FreeErrorData(edata); } diff --git a/src/pl/tcl/sql/pltcl_setup.sql b/src/pl/tcl/sql/pltcl_setup.sql new file mode 100644 index 53358ea..ddc30c9 *** a/src/pl/tcl/sql/pltcl_setup.sql --- b/src/pl/tcl/sql/pltcl_setup.sql *************** drop table foo; *** 595,597 **** --- 595,613 ---- drop event trigger tcl_a_snitch; drop event trigger tcl_b_snitch; + + + -- test error handling + CREATE OR REPLACE FUNCTION pg_temp.tcl_eval (varchar) RETURNS varchar AS $$ + eval $1 + $$ LANGUAGE pltcl; + + select pg_temp.tcl_eval('spi_exec "select * from foo;"'); + select pg_temp.tcl_eval($$ + set list [lindex $::errorCode 0]; + foreach "key value" [lrange $::errorCode 1 end] { + if {$key == "domain" || $key == "context_domain" || $key == "lineno"} continue; + lappend list $key $value + }; + return [join $list "\n"] + $$);
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers