Re: Improve error handling in pltcl

From: Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Improve error handling in pltcl
Date: 2016-03-13 19:24:38
Message-ID: 56E5BE76.2000100@BlueTreble.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
pltcl_error_master-2.patch text/plain 13.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jim Nasby 2016-03-13 19:52:03 Re: Sanity checking for ./configure options?
Previous Message Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?= 2016-03-13 19:10:10 [PATCH] Obsolete wording in PL/Perl comment