Jim Nasby <jim.na...@bluetreble.com> 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