On 3/17/16 5:46 PM, Tom Lane wrote:
Pavel Stehule <pavel.steh...@gmail.com> writes:
I'll mark this patch as ready for commiters.
I started to look at this patch. It seems to me that the format of the
errorCode output is not terribly well designed.
...
Maybe there's another way. I've not used Tcl in anger since around
the turn of the century, so it's entirely likely that I'm missing
something. But the proposed doc addition isn't showing me any really
easy way to work with this data format, and I think that that's either
a design fail or a docs fail, not something I should just accept as
the best we can do.
I asked Karl about this (since he's active in the TCL community and
works with TCL every day), and his response was essentially:
Tcl is all about flat lists of key value pairs.
array set myArray $list
sucks a flat list of key-value pairs into an array and vice versa
set list [array get myArray]
creates one. This is normal Tcl stuff.
Getting the errorCode into an array is as easy as
array set errorData [lrange $errorCode 1 end]
Then you can do
$errorData(detail), $errorData(message), etc.
In fact keyed lists in TclX which are the inspiration for the approach
to lists of alternating key-value pairs did it the way he suggested and
that’s fallen by the wayside in favor of flat lists.
There has been a formal proposal to add a -stride to lsearch to make
lsearch efficient at searching the same flat lists of key-value pairs
and I expect to see it in Tcl 8.7 or sooner.
The doc example also makes me think that more effort should get expended
on converting internalquery/internalpos to just be query/cursorpos.
It seems unlikely to me that a Tcl function could ever see a case
where the latter fields are useful directly.
Is there docs or an example on how to handle that? I looked at the
plpython stuff and I'm still really unclear on what exactly an
internalquery is as opposed to regular context info?
PLy_spi_exception_set simply exposes the raw internalquery and internalpos.
Also, I'm curious as to why you think "domain" or "context_domain"
is of any value to expose here. Tcl code is not going to have any
access to the NLS infrastructure (if that's even been compiled) to
do anything with those values.
I'm not really sure what it's hurting to expose that, but I'll remove it.
And I believe it may be a security violation for this code to expose
"detail_log". The entire point of that field is it goes to the
postmaster log and NOT anywhere where unprivileged clients can see it.
Removed.
Nitpickier stuff:
* Docs example could use work: it should show how to do something
useful *in Tcl code*, like maybe checking whether an error had a
particular SQLSTATE. The example with dumping the whole list at the
psql command line is basically useless, not to mention that it
relies on a nonexistent "tcl_eval" function. (And I don't care
Will work on an improved example.
for the regression test case creating such a function ... isn't
that a fine SQL-injection hole?)
If it was taking external input, but it's not, and it saves from
creating 2 separate functions (which you want to do to make sure the
global errorCode is being set, and not a local copy).
* I believe pltcl_construct_errorCode needs to do E2U encoding
conversion for anything that could contain non-ASCII data, which is
most of the non-fixed strings.
Done.
* Useless-looking hunk at pltcl.c:1610
Removed.
* I think the unstable data you're griping about is the Tcl function's
OID, not the PID. (I wonder whether we should make an effort to hide
that in errorInfo. But if so it's a matter for a separate patch.)
It's possible that someone would want to know the name of the
constructed TCL function (and yeah, I think it's the OID not PID).
I'll set this patch back to Waiting On Author. I believe it's well
within reach of getting committed in this fest, but it needs more
work.
Interim patch attached (need to work on the docs).
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
diff --git a/doc/src/sgml/pltcl.sgml b/doc/src/sgml/pltcl.sgml
index d2175d5..d5c576d 100644
--- a/doc/src/sgml/pltcl.sgml
+++ b/doc/src/sgml/pltcl.sgml
@@ -775,6 +775,127 @@ CREATE EVENT TRIGGER tcl_a_snitch ON ddl_command_start
EXECUTE PROCEDURE tclsnit
</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
index e11718c..9a9d7d1 100644
--- a/src/pl/tcl/expected/pltcl_setup.out
+++ b/src/pl/tcl/expected/pltcl_setup.out
@@ -555,3 +555,47 @@ NOTICE: tclsnitch: ddl_command_start DROP TABLE
NOTICE: tclsnitch: ddl_command_end DROP TABLE
drop event trigger tcl_a_snitch;
drop event trigger tcl_b_snitch;
+-- test error handling
+/*
+ * The ugly hack of messsing with the verbosity is because the error context is
+ * set to the TCL variable errorInfo, which contains some unstable data (namely
+ * the full name of the TCL function created by the handler, which includes the
+ * Postgres backend PID).
+ */
+\set VERBOSITY terse
+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
+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"} {
+ regsub -all {[0-9]} $value "" value
+ }
+ lappend list $key $value
+};
+return [join $list "\n"]
+$$);
+ tcl_eval
+-------------------------------
+ POSTGRES +
+ SQLSTATE +
+ 42P01 +
+ message +
+ relation "foo" does not exist+
+ cursor_position +
+ 0 +
+ internalquery +
+ select * from foo; +
+ internal_position +
+ 15 +
+ filename +
+ parse_relation.c +
+ lineno +
+ +
+ funcname +
+ parserOpenTable
+(1 row)
+
diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c
index 5b27c73..1c7678f 100644
--- a/src/pl/tcl/pltcl.c
+++ b/src/pl/tcl/pltcl.c
@@ -1576,6 +1576,85 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid,
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();
+
+ UTF_BEGIN;
+ Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("POSTGRES", -1));
+ Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("SQLSTATE", -1));
+ Tcl_ListObjAppendElement(interp, obj,
Tcl_NewStringObj(unpack_sql_state(edata->sqlerrcode), -1));
+ Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("message", -1));
+ Tcl_ListObjAppendElement(interp, obj,
Tcl_NewStringObj(UTF_E2U(edata->message), -1));
+
+ if (edata->detail)
+ {
+ Tcl_ListObjAppendElement(interp, obj,
Tcl_NewStringObj("detail", -1));
+ Tcl_ListObjAppendElement(interp, obj,
Tcl_NewStringObj(UTF_E2U(edata->detail), -1));
+ }
+ if (edata->hint)
+ {
+ Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("hint",
-1));
+ Tcl_ListObjAppendElement(interp, obj,
Tcl_NewStringObj(UTF_E2U(edata->hint), -1));
+ }
+ if (edata->context)
+ {
+ Tcl_ListObjAppendElement(interp, obj,
Tcl_NewStringObj("context", -1));
+ Tcl_ListObjAppendElement(interp, obj,
Tcl_NewStringObj(UTF_E2U(edata->context), -1));
+ }
+ if (edata->schema_name)
+ {
+ Tcl_ListObjAppendElement(interp, obj,
Tcl_NewStringObj("schema", -1));
+ Tcl_ListObjAppendElement(interp, obj,
Tcl_NewStringObj(UTF_E2U(edata->schema_name), -1));
+ }
+ if (edata->table_name)
+ {
+ Tcl_ListObjAppendElement(interp, obj, Tcl_NewStringObj("table",
-1));
+ Tcl_ListObjAppendElement(interp, obj,
Tcl_NewStringObj(UTF_E2U(edata->table_name), -1));
+ }
+ if (edata->column_name)
+ {
+ Tcl_ListObjAppendElement(interp, obj,
Tcl_NewStringObj("column", -1));
+ Tcl_ListObjAppendElement(interp, obj,
Tcl_NewStringObj(UTF_E2U(edata->column_name), -1));
+ }
+ if (edata->datatype_name)
+ {
+ Tcl_ListObjAppendElement(interp, obj,
Tcl_NewStringObj("datatype", -1));
+ Tcl_ListObjAppendElement(interp, obj,
Tcl_NewStringObj(UTF_E2U(edata->datatype_name), -1));
+ }
+ if (edata->constraint_name)
+ {
+ Tcl_ListObjAppendElement(interp, obj,
Tcl_NewStringObj("constraint", -1));
+ Tcl_ListObjAppendElement(interp, obj,
Tcl_NewStringObj(UTF_E2U(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(UTF_E2U(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(UTF_E2U(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(UTF_E2U(edata->funcname), -1));
+ }
+ UTF_END;
+
+ Tcl_SetObjErrorCode(interp, obj);
+}
+
/**********************************************************************
* pltcl_elog() - elog() support for PLTcl
@@ -1652,6 +1731,7 @@ pltcl_elog(ClientData cdata, Tcl_Interp *interp,
UTF_BEGIN;
Tcl_SetObjResult(interp,
Tcl_NewStringObj(UTF_E2U(edata->message), -1));
UTF_END;
+ pltcl_construct_errorCode(interp, edata);
FreeErrorData(edata);
return TCL_ERROR;
@@ -1884,6 +1964,7 @@ pltcl_subtrans_abort(Tcl_Interp *interp,
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
index 53358ea..3ee8583 100644
--- a/src/pl/tcl/sql/pltcl_setup.sql
+++ b/src/pl/tcl/sql/pltcl_setup.sql
@@ -595,3 +595,29 @@ drop table foo;
drop event trigger tcl_a_snitch;
drop event trigger tcl_b_snitch;
+
+
+-- test error handling
+
+/*
+ * The ugly hack of messsing with the verbosity is because the error context is
+ * set to the TCL variable errorInfo, which contains some unstable data (namely
+ * the full name of the TCL function created by the handler, which includes the
+ * Postgres backend PID).
+ */
+\set VERBOSITY terse
+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"} {
+ regsub -all {[0-9]} $value "" value
+ }
+ 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
