Re: Improve error handling in pltcl

On 3/17/16 5:46 PM, Tom Lane wrote:
> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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