Re: [HACKERS] Improve error handling in pltcl
On 3/25/16 3:11 PM, Tom Lane wrote: Jim Nasby writes: the data, we're making it unnecessarily hard. All we need is one more field in there, and you can simplify that to Ahh, nice. I think actually it's a simple point: there won't ever be a case where cursorpos is set here, because that's only used for top-level SQL syntax errors. Anything we are catching would be an internal-query error, so we might as well not confuse PL/Tcl users with the distinction but just report internalquery/internalpos as the statement and cursor position. PLy_spi_exception_set simply exposes the raw internalquery and internalpos. Right, because that's all that could be useful. Ahh, ok, finally I get it. It would be nice if the comments for ErrorData were clearer... it strikes me that this is not coding style we want to encourage. We should borrow the infrastructure plpgsql has for converting SQLSTATEs into condition names, so that that can be more like Yeah, Karl and I were just talking about that as we were finishing up the docs changes (ironically, as you were commiting this...). I ended up with a more realistic example that also demonstrates that you can refer to errorCode in a separate function if desired. That patch attached for posterity. -- 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..4cf32df 100644 --- a/doc/src/sgml/pltcl.sgml +++ b/doc/src/sgml/pltcl.sgml @@ -775,6 +775,169 @@ CREATE EVENT TRIGGER tcl_a_snitch ON ddl_command_start EXECUTE PROCEDURE tclsnit + +Error Handling in PL/Tcl + + + error handling + in PL/Tcl + + + + All Tcl errors that occur within a stored procedure and are not caught + using Tcl's catch or try + functions will raise a database error. + + + Tcl code can raise a database error by invoking the + elog command provided by PL/Tcl or by generating an + error using the Tcl error command and not catching it + with Tcl's catch command. + + + Database errors that occur from the PL/Tcl stored procedure's + use of spi_exec, spi_prepare, + and spi_execp are also catchable by Tcl's + catch command. + + + Tcl provides an errorCode variable that can represent + additional information about the error in a form that is easy for programs + to interpret. The contents are a Tcl list format. The first word + identifies the subsystem or library responsible for the error. The + remaining contents are up to the individual code or library. For example + if Tcl's open command is asked to open a file that + doesn't exist, errorCode might contain POSIX + ENOENT {no such file or directory} where the third element may + vary by locale but the first and second will not. + + + When spi_exec, spi_prepare + or spi_execp cause a database error to be raised, + that database eror propagates back to Tcl as a Tcl error. In this case + errorCode is set to a list where the first element is + POSTGRES followed by details of the Postgres error. + 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 + errorCode list are key-value pairs where the first + value is the name of the field and the second is its value. + + + Fields that may be present include SQLSTATE, + message, + detail, + hint, + context, + schema, + table, + column, + datatype, + constraint, + cursor_position, + internalquery, + internal_position, + filename, + lineno and + funcname. + + + You might find it useful to load the results into an array. Code + for doing that might look like + +if {[lindex $errorCode 0] == "POSTGRES"} { +array set errorRow [lrange $errorCode 1 end] +} + + + + This example shows how to trap a specific SQL error. + +CREATE TABLE account(user_name varchar(1) NOT NULL PRIMARY KEY); +CREATE OR REPLACE FUNCTION public.create_user(user_name text) + RETURNS void LANGUAGE pltcl AS $function$ +set prep [ spi_prepare "INSERT INTO account(user_name) VALUES(\$1)" [ list text ] ] +if [ catch { +spi_execp $prep [ list $1 ] +} msg ] { +if {[lindex $::errorCode 0] == "POSTGRES"} { +array set errorData [lrange $::errorCode 1 end] +if { $errorData(SQLSTATE) == "23505" && $errorData(constraint) == "account_pkey" } { +return -code error "user '$1' already exists" +} +} +throw $::errorCode $msg +} +$function$; + + + +SELECT create_user('a'); + create_user +- + +(1 row) + +SELECT create_user('a'); +ERROR: user 'a' already exists +
Re: [HACKERS] Improve error handling in pltcl
Jim Nasby writes: > On 3/17/16 5:46 PM, Tom Lane wrote: >> I started to look at this patch. It seems to me that the format of the >> errorCode output is not terribly well designed. > Getting the errorCode into an array is as easy as > array set errorData [lrange $errorCode 1 end] Well, my point is that if we expect that this is *the* way to work with the data, we're making it unnecessarily hard. All we need is one more field in there, and you can simplify that to array set errorData $errorCode which is less typing, less opportunity for mistyping, and fewer machine cycles. I figure we can stick the Postgres version in after POSTGRES and nobody will think that particularly odd, but it enables simpler loading of the results into an array. >> 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 think actually it's a simple point: there won't ever be a case where cursorpos is set here, because that's only used for top-level SQL syntax errors. Anything we are catching would be an internal-query error, so we might as well not confuse PL/Tcl users with the distinction but just report internalquery/internalpos as the statement and cursor position. > PLy_spi_exception_set simply exposes the raw internalquery and internalpos. Right, because that's all that could be useful. >> * 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. Nah, you need a separate UTF_BEGIN/END pair for each one, else you're leaking all but the last translated string. I'm not entirely sure that it's worth the trouble to avoid such transient leaks, but as long as PL/Tcl has got this infrastructure we should use it. Anyway, I cleaned all that up and committed it, but as I'm sitting here looking at the docs example I used: if {$errorArray(SQLSTATE) == "42P01"} { # UNDEFINED_TABLE it strikes me that this is not coding style we want to encourage. We should borrow the infrastructure plpgsql has for converting SQLSTATEs into condition names, so that that can be more like if {$errorArray(condition) == "undefined_table"} { Think I'll go fix that before I leave this subject. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Improve error handling in pltcl
On 3/20/16 8:42 PM, Jim Nasby wrote: 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. Anyone any pointers on this? I'm not sure I can finish the docs without knowing what we want to do here. -- 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Improve error handling in pltcl
On 3/17/16 5:46 PM, Tom Lane wrote: Pavel Stehule 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 + +Error Handling in PL/Tcl + + + error handling + in PL/Tcl + + + + 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 catch command will raise a database + error. + + + Tcl code within or called from the stored procedure can choose to + raise a database error by invoking the elog + command provided by PL/Tcl or by generating an error using the Tcl + error command and not catching it with Tcl's +
Re: [HACKERS] Improve error handling in pltcl
Pavel Stehule 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. I see that Tcl constrains it to be a list starting with an error-class-identifying keyword, for which you've chosen POSTGRES. So far fine. But for the rest of the list, you've chosen a format of alternating keywords and values, wherein some of the pairs may be missing, and (I presume) we reserve the right to invent new keywords. Admittedly my Tcl is pretty rusty, but this seems to me to be not easy to deal with. The errorCode list format is really designed on the assumption that any particular error-class-identifying keyword implies a fixed format for the rest of the list, so you can pull out the fields you care about with lindex. But here, users cannot safely assume that any particular value is at a specific list index, which means they've got to search for the keyword they want, and this representation doesn't make that very easy AFAICS. The doc text proposes loading all but the POSTGRES keyword into a Tcl array, which solves the problem since the keywords become knowable array subscripts, but even that is pretty awkward because you've got to leave out POSTGRES. (Also, using the array is a bit awkward if you aren't sure whether an entry is present, no?) So I think this needs to be redesigned to make it less painful to pull out the value for a given keyword. I'm not very clear on what's the best way, though. One line of thought is to make the format use sublists: POSTGRES {keyword value} {keyword value} ... This could be searched with lsearch, eg to get the SQLSTATE lindex [lsearch -index 0 -inline $errorCode SQLSTATE] 1 but that still seems pretty awkward (maybe there's a better way?) Another idea is to put some junk value after POSTGRES (maybe the server version number?) with the idea that then it would be trivial to load the data into an array with "array set". But you'd really want to document it as that's what you MUST do to extract data, not that it's an optional solution. 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. 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. 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. 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. 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 for the regression test case creating such a function ... isn't that a fine SQL-injection hole?) * 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. * Useless-looking hunk at pltcl.c:1610 * 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.) 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. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Improve error handling in pltcl
Hi 2016-03-13 20:24 GMT+01:00 Jim Nasby : > On 3/3/16 8:51 AM, Pavel Stehule wrote: > >> Hi >> >> I am testing behave, and some results looks strange >> > > Thanks for the review! > > postgres=# \sf foo >> CREATE OR REPLACE FUNCTION public.foo() >> RETURNS void >> LANGUAGE plpgsql >> AS $function$ >> begin >>raise exception sqlstate 'ZZ666' using message='hello, world', >> detail='hello, my world', hint = 'dont afraid'; >> end >> $function$ >> >> postgres=# select tcl_eval('spi_exec "select foo();"'); >> ERROR: 38000: hello, world >> CONTEXT: hello, world ==??? >> >> the message was in context. Probably it is out of scope of this patch, >> but it isn't consistent with other PL >> >> >> while executing >> "spi_exec "select foo();"" >> ("eval" body line 1) >> invoked from within >> "eval $1" >> (procedure "__PLTcl_proc_16864" line 3) >> invoked from within >> "__PLTcl_proc_16864 {spi_exec "select foo();"}" >> in PL/Tcl function "tcl_eval" >> LOCATION: throw_tcl_error, pltcl.c:1217 >> Time: 1.178 ms >> > > Both problems actually exists in HEAD. The issue is this line in > throw_tcl_error: > > econtext = utf_u2e(Tcl_GetVar(interp, "errorInfo", > TCL_GLOBAL_ONLY)); > > Offhand I don't see any great way to improve that behavior, and in any > case it seems out of scope for this patch. As a workaround I'm just forcing > psql error VERBOSITY to terse for now. > I understand - it is unpleasant, but it is another scope. > > postgres=# select tcl_eval('join $::errorCode "\n"'); >> tcl_eval >> ═ >> POSTGRES ↵ >> message↵ >> hello, world ↵ >> detail ↵ >> hello, my world↵ >> hint ↵ >> dont afraid↵ >> domain ↵ >> plpgsql-9.6↵ >> context_domain ↵ >> postgres-9.6 ↵ >> context↵ >> PL/pgSQL function foo() line 3 at RAISE↵ >> SQL statement "select foo();" ↵ >> cursor_position↵ >> 0 ↵ >> filename ↵ >> pl_exec.c ↵ >> lineno ↵ >> 3165 ↵ >> funcname ↵ >> exec_stmt_raise >> (1 row) >> >> I miss a SQLSTATE. >> > > Great catch. Fixed. > I can verify it. The doc should be updated too. > > Why is used List object instead dictionary? TCL supports it >> https://www.tcl.tk/man/tcl8.5/tutorial/Tcl23a.html >> > > Because errorCode unfortunately is an array and not a dict. It doesn't > really seem worth messing with it in the eval since this is just a sanity > check... > I am sorry, my I expected so we introduced errorCode. My question was not valid > New patch attached. It also removes some other unstable output from the > regression test. > I checked this patch: * This patch is relative trivial without any controversy - allow a access to ErrorData fields is good idea, and we do it in some other PL longer time. * There are no problem with patching, compiling * all tests was passed * a comments in code are adequate to low complexity * code respects PostgreSQL formatting * attached documentation is good and correct * regress tests are adequate I fixed the documentation - there was not information about SQLSTATE field. See, please, attachment. I'll mark this patch as ready for commiters. Regards Pavel > -- > 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 new file mode 100644 index d2175d5..1ac7804 *** a/doc/src/sgml/pltcl.sgml --- b/doc/src/sgml/pltcl.sgml *** CREATE EVENT TRIGGER tcl_a_snitch ON ddl *** 775,780 --- 775,903 + + Error Handling in PL/Tcl + + + error handling + in PL/Tcl + + + + 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 catch command will raise a database + error. + + + Tcl code within or called from the stored procedure can choose to + raise a database error by invoking the elog + command provided by PL/Tcl or by generating an error using the Tcl + error command and not catching it with Tcl's + catch command. + + + Database errors that occur from the PL/Tcl stored procedure's + use of spi_exec, spi_prepare, + and spi_execp are al
Re: [HACKERS] Improve error handling in pltcl
On 3/3/16 8:51 AM, Pavel Stehule wrote: Hi I am testing behave, and some results looks strange Thanks for the review! postgres=# \sf foo CREATE OR REPLACE FUNCTION public.foo() RETURNS void LANGUAGE plpgsql AS $function$ begin raise exception sqlstate 'ZZ666' using message='hello, world', detail='hello, my world', hint = 'dont afraid'; end $function$ postgres=# select tcl_eval('spi_exec "select foo();"'); ERROR: 38000: hello, world CONTEXT: hello, world ==??? the message was in context. Probably it is out of scope of this patch, but it isn't consistent with other PL while executing "spi_exec "select foo();"" ("eval" body line 1) invoked from within "eval $1" (procedure "__PLTcl_proc_16864" line 3) invoked from within "__PLTcl_proc_16864 {spi_exec "select foo();"}" in PL/Tcl function "tcl_eval" LOCATION: throw_tcl_error, pltcl.c:1217 Time: 1.178 ms Both problems actually exists in HEAD. The issue is this line in throw_tcl_error: econtext = utf_u2e(Tcl_GetVar(interp, "errorInfo", TCL_GLOBAL_ONLY)); Offhand I don't see any great way to improve that behavior, and in any case it seems out of scope for this patch. As a workaround I'm just forcing psql error VERBOSITY to terse for now. postgres=# select tcl_eval('join $::errorCode "\n"'); tcl_eval ═ POSTGRES ↵ message↵ hello, world ↵ detail ↵ hello, my world↵ hint ↵ dont afraid↵ domain ↵ plpgsql-9.6↵ context_domain ↵ postgres-9.6 ↵ context↵ PL/pgSQL function foo() line 3 at RAISE↵ SQL statement "select foo();" ↵ cursor_position↵ 0 ↵ filename ↵ pl_exec.c ↵ lineno ↵ 3165 ↵ funcname ↵ exec_stmt_raise (1 row) I miss a SQLSTATE. Great catch. Fixed. Why is used List object instead dictionary? TCL supports it https://www.tcl.tk/man/tcl8.5/tutorial/Tcl23a.html Because errorCode unfortunately is an array and not a dict. It doesn't really seem worth messing with it in the eval since this is just a sanity check... New patch attached. It also removes some other unstable output from the regression test. -- 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 + +Error Handling in PL/Tcl + + + error handling + in PL/Tcl + + + + 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 catch command will raise a database + error. + + + Tcl code within or called from the stored procedure can choose to + raise a database error by invoking the elog + command provided by PL/Tcl or by generating an error using the Tcl + error command and not catching it with Tcl's + catch command. + + + Database errors that occur from the PL/Tcl stored procedure's + use of spi_exec, spi_prepare, + and spi_execp are also catchable by Tcl's + catch command. + + + Tcl provides an errorCode 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 + open command is asked to open a file that doesn't + exist, errorCode + might contain POSIX ENOENT {no such file or directory} + where the third element may vary by locale but the first and second + will not. + + + When spi_exec, spi_prepare + or spi_execp cause a database error to be raised, + that database eror propagates back to Tcl as a Tcl error. + In this case errorCode is set to a list + where the first element is POSTGRES followed by a + copious decoding of the Postgres error structure. Since fields in the + structure may or may not be present depending on t
Re: [HACKERS] Improve error handling in pltcl
On Thu, Mar 3, 2016 at 9:51 AM, Pavel Stehule wrote: > I am testing behave, and some results looks strange Jim, this is waiting for you to respond to Pavel's review. If that doesn't happen soon, this should be marked Returned with Feedback and you can, if you wish, resubmit for 9.7. Thanks, -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Improve error handling in pltcl
Hi I am testing behave, and some results looks strange postgres=# \sf foo CREATE OR REPLACE FUNCTION public.foo() RETURNS void LANGUAGE plpgsql AS $function$ begin raise exception sqlstate 'ZZ666' using message='hello, world', detail='hello, my world', hint = 'dont afraid'; end $function$ postgres=# select tcl_eval('spi_exec "select foo();"'); ERROR: 38000: hello, world CONTEXT: hello, world ==??? the message was in context. Probably it is out of scope of this patch, but it isn't consistent with other PL while executing "spi_exec "select foo();"" ("eval" body line 1) invoked from within "eval $1" (procedure "__PLTcl_proc_16864" line 3) invoked from within "__PLTcl_proc_16864 {spi_exec "select foo();"}" in PL/Tcl function "tcl_eval" LOCATION: throw_tcl_error, pltcl.c:1217 Time: 1.178 ms postgres=# select tcl_eval('join $::errorCode "\n"'); tcl_eval ═ POSTGRES ↵ message↵ hello, world ↵ detail ↵ hello, my world↵ hint ↵ dont afraid↵ domain ↵ plpgsql-9.6↵ context_domain ↵ postgres-9.6 ↵ context↵ PL/pgSQL function foo() line 3 at RAISE↵ SQL statement "select foo();" ↵ cursor_position↵ 0 ↵ filename ↵ pl_exec.c ↵ lineno ↵ 3165 ↵ funcname ↵ exec_stmt_raise (1 row) I miss a SQLSTATE. Why is used List object instead dictionary? TCL supports it https://www.tcl.tk/man/tcl8.5/tutorial/Tcl23a.html Regards Pavel
Re: [HACKERS] Improve error handling in pltcl
Hi 2016-03-01 22:23 GMT+01:00 Jim Nasby : > On 2/29/16 10:01 PM, Tom Lane wrote: > >> Jim Nasby 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_ 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 + + Error Handling in PL/Tcl + + + error handling + in PL/Tcl + + + + 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 catch command will raise a database + error. + + + Tcl code within or called from the stored procedure can choose to + raise a database error by invoking the elog + command provided by PL/Tcl or by generating an error using the Tcl + error command and not catching it with Tcl's + catch command. + + + Database errors that occur from the PL/Tcl stored procedure's + use of spi_exec, spi_prepare, + and spi_execp are also catchable by Tcl's + catch command. + + + Tcl provides an errorCode 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 + open command is asked to open a file that doesn't + exist, errorCode + might contain POSIX ENOENT {no such file or directory} + where the third element may vary by locale but the first and second + will not. + + + When spi_exec, spi_prepare + or spi_execp cause a database error to be raised, + that database eror propagates back to Tcl as a Tcl error. + In this case errorCode is set to a list + where the first element is POSTGRES 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 errorCode + list are key-value pairs where the first value is the name of the + field and the second is its value. + + + Fields that may be present include message, + detail, detail_log, + hint, domain, + context_domain, context, + schema, table, + column, datatype, + constraint, cursor_position, + internalquery, internal_position, + filename, lineno and + funcname. + + + You might find it useful to load the results into an array. Code + for doing that might look like + + if {[lindex $errorCode 0] == "POSTGRES"} { + array set errorRow [lrange $errorCode 1 end] + } + + + + In the example below we cause an error by attempting to + SELECT from a table that doesn't exist. + + select tcl_eval('spi_exec "select * from foo;"'); + + + + ERROR: relation "foo" does not exist + + + + + Now we examine the error code. (The d
Re: [HACKERS] Improve error handling in pltcl
On 2/29/16 10:01 PM, Tom Lane wrote: Jim Nasby 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! Heh, I didn't realize you were a TCL fan. They've been heavy PG users from the start. Eventually PG had trouble keeping up with the in-flight tracking so they created Speed Tables [1]. And Karl (one of the founders) is a well known TCL contributor[2]. When it comes to sheer geek factor though, I think the multilateration[3] stuff they're doing with their ADS-B network is a really cool data application. It's basically a form of "reverse GPS" for tracking aircraft. [1] https://github.com/flightaware/speedtables [2] http://wiki.tcl.tk/83 [3] http://flightaware.com/adsb/mlat/ -- 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Improve error handling in pltcl
Jim Nasby 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! regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Improve error handling in pltcl
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). -- 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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Improve error handling in pltcl
Per discussion in [1], this patch improves error reporting in pltcl. pltcl_error_objects.patch applies on top of the pltcl_objects_2.patch referenced in [2]. pltcl_error_master.patch applies against current master. [1] http://www.postgresql.org/message-id/20160223150401.2173d...@wagner.wagner.home [2] http://www.postgresql.org/message-id/56cce7d2.9090...@bluetreble.com -- 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 + +Error Handling in PL/Tcl + + + error handling + in PL/Tcl + + + + 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 catch command will raise a database + error. + + + Tcl code within or called from the stored procedure can choose to + raise a database error by invoking the elog + command provided by PL/Tcl or by generating an error using the Tcl + error command and not catching it with Tcl's + catch command. + + + Database errors that occur from the PL/Tcl stored procedure's + use of spi_exec, spi_prepare, + and spi_execp are also catchable by Tcl's + catch command. + + + Tcl provides an errorCode 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 + open command is asked to open a file that doesn't + exist, errorCode + might contain POSIX ENOENT {no such file or directory} + where the third element may vary by locale but the first and second + will not. + + + When spi_exec, spi_prepare + or spi_execp cause a database error to be raised, + that database eror propagates back to Tcl as a Tcl error. + In this case errorCode is set to a list + where the first element is POSTGRES 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 errorCode + list are key-value pairs where the first value is the name of the + field and the second is its value. + + + Fields that may be present include message, + detail, detail_log, + hint, domain, + context_domain, context, + schema, table, + column, datatype, + constraint, cursor_position, + internalquery, internal_position, + filename, lineno and + funcname. + + + You might find it useful to load the results into an array. Code + for doing that might look like + +if {[lindex $errorCode 0] == "POSTGRES"} { +array set errorRow [lrange $errorCode 1 end] +} + + + + In the example below we cause an error by attempting to + SELECT from a table that doesn't exist. + +select tcl_eval('spi_exec "select * from foo;"'); + + + +ERROR: relation "foo" does not exist + + + + + Now we examine the error code. (The double-colons explicitly + reference errorCode as a global variable.) + +select tcl_eval('join $::errorCode "\n"'); + + + + 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) + + + + + Modules and the unknown Command diff --git a/src/pl/tcl/expected/pltcl_setup.out b/src/pl/tcl/expected/pltcl_setup.out index 4183c14..0a9f9f4 100644 --- a/src/pl/tcl/expected/pltcl_setup.out +++ b/src/pl/tcl/expected/pltcl_setup.out @@ -542,3 +542,44 @@ 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 erro