Re: [HACKERS] proposal: use errcontext for custom exception too

2011-11-25 Thread Pavel Stehule
2011/11/25 Tom Lane :
> Robert Haas  writes:
>> On Thu, Nov 24, 2011 at 12:30 PM, Pavel Stehule  
>> wrote:
>>> There are small issue in PL/pgSQL and custom exceptions. Custom
>>> exception doesn't set a CONTEXT field. I propose change this behave
>>> for WARNING or EXCEPTION level. The goal is same behave for custom
>>> exception and builtin exception and it can help to identify a RAISE
>>> statement that is responsible to exception.
>
>> That seems completely arbitrary.  I think we discussed before
>> providing an option to allow the user to control this, which seems
>> better than implementing some hardcoded rule that may or may not be
>> what a given user wants.
>
> Note also that the current behavior *is* what people want; at least,
> we have seen no field complaints about the lack of first-level CONTEXT
> for RAISE notices, and plenty of complaints from people who think
> there's still too much cruft automatically attached to RAISE output.
> If anything, what's been requested is a way to suppress even more
> context, not a policy decision to force more of it.
>

People usually don't like verbose output in interactive mode in
console. CONTEXT for RAISE NOTICE is not necessary.  If you have a
small functions, then CONTEXT for RAISE EXCEPTION is not necessary
too. But if you have a functions with hundreds lines, then more
informations about origin of exception is welcome. There is workaround
- with one statement function (RAISE stmt wrapper) I have a expected
behave - but it's not clean RAISE EXCEPTION 'some message' is more
readable than PERFORM elog('some message', ..) and log is not too
readable too.

postgres=# SELECT yyy();
CONTEXT:  SQL statement "SELECT xxx()"
PL/pgSQL function "yyy" line 3 at PERFORM

I can understand to motivation decrease verbosity, but there is clean
request "simply identification a source of exception (exception, not
notification)". Some RAISE stmt option should be - but for NOTICE
level NO_CONTEXT is optimal, and for EXCEPTION NO_CONTEXT is
suboptimal. It has sense just for WARNING level.

Regards

Pavel

-- 
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] proposal: use errcontext for custom exception too

2011-11-25 Thread Tom Lane
Robert Haas  writes:
> On Thu, Nov 24, 2011 at 12:30 PM, Pavel Stehule  
> wrote:
>> There are small issue in PL/pgSQL and custom exceptions. Custom
>> exception doesn't set a CONTEXT field. I propose change this behave
>> for WARNING or EXCEPTION level. The goal is same behave for custom
>> exception and builtin exception and it can help to identify a RAISE
>> statement that is responsible to exception.

> That seems completely arbitrary.  I think we discussed before
> providing an option to allow the user to control this, which seems
> better than implementing some hardcoded rule that may or may not be
> what a given user wants.

Note also that the current behavior *is* what people want; at least,
we have seen no field complaints about the lack of first-level CONTEXT
for RAISE notices, and plenty of complaints from people who think
there's still too much cruft automatically attached to RAISE output.
If anything, what's been requested is a way to suppress even more
context, not a policy decision to force more of it.

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] proposal: use errcontext for custom exception too

2011-11-25 Thread Robert Haas
On Fri, Nov 25, 2011 at 1:14 AM, Pavel Stehule  wrote:
> A some option via #option or GUC has sense for lower levels like
> NOTICE or WARNING.

I think what we discussed before was adding some bit of optional
syntax to RAISE that would indicate that the user wants CONTEXT
suppressed.

-- 
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] proposal: use errcontext for custom exception too

2011-11-24 Thread Pavel Stehule
2011/11/25 Robert Haas :
> On Thu, Nov 24, 2011 at 12:30 PM, Pavel Stehule  
> wrote:
>> There are small issue in PL/pgSQL and custom exceptions. Custom
>> exception doesn't set a CONTEXT field. I propose change this behave
>> for WARNING or EXCEPTION level. The goal is same behave for custom
>> exception and builtin exception and it can help to identify a RAISE
>> statement that is responsible to exception.
>
> That seems completely arbitrary.  I think we discussed before
> providing an option to allow the user to control this, which seems
> better than implementing some hardcoded rule that may or may not be
> what a given user wants.

A some option via #option or GUC has sense for lower levels like
NOTICE or WARNING. For exception level CONTEXT should be filled every
time - usually you have a stack of CONTEXT calls, because exception
must not be on direct call, but the last CONTEXT (where exception was
created missing). It is confusing. When a advanced developer see a
exception without CONTEXT, then he know so exception is related to
RAISE statement, but still is not simple find a statement, that raised
exception - the line number is missing.

Compromise solution can be GUC where CONTEXT is default for ERROR level

like plpgsql.log_context_error_level = ERROR

A new option on RAISE STATEMENT is not well, usually you want to same
behave on complete application.

Regards

Pavel

>
> --
> 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] proposal: use errcontext for custom exception too

2011-11-24 Thread Robert Haas
On Thu, Nov 24, 2011 at 12:30 PM, Pavel Stehule  wrote:
> There are small issue in PL/pgSQL and custom exceptions. Custom
> exception doesn't set a CONTEXT field. I propose change this behave
> for WARNING or EXCEPTION level. The goal is same behave for custom
> exception and builtin exception and it can help to identify a RAISE
> statement that is responsible to exception.

That seems completely arbitrary.  I think we discussed before
providing an option to allow the user to control this, which seems
better than implementing some hardcoded rule that may or may not be
what a given user wants.

-- 
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


[HACKERS] proposal: use errcontext for custom exception too

2011-11-24 Thread Pavel Stehule
Hello

There are small issue in PL/pgSQL and custom exceptions. Custom
exception doesn't set a CONTEXT field. I propose change this behave
for WARNING or EXCEPTION level. The goal is same behave for custom
exception and builtin exception and it can help to identify a RAISE
statement that is responsible to exception.


./pl_exec.c
*** ./pl_exec.c.orig2011-11-24 17:29:08.0 +0100
--- ./pl_exec.c 2011-11-24 18:23:51.513136718 +0100
***
*** 2827,2833 
/*
 * Throw the error (may or may not come back)
 */
!   estate->err_text = raise_skip_msg;  /* suppress traceback of raise 
*/

ereport(stmt->elog_level,
(err_code ? errcode(err_code) : 0,
--- 2827,2834 
/*
 * Throw the error (may or may not come back)
 */
!   if (stmt->elog_level < WARNING)
!   estate->err_text = raise_skip_msg;  /* suppress traceback 
of raise notice */

ereport(stmt->elog_level,
(err_code ? errcode(err_code) : 0,

Regards

Pavel Stehule
*** ./src/pl/plpgsql/src/pl_exec.c.orig	2011-11-24 17:29:08.0 +0100
--- ./src/pl/plpgsql/src/pl_exec.c	2011-11-24 18:23:51.513136718 +0100
***
*** 2827,2833 
  	/*
  	 * Throw the error (may or may not come back)
  	 */
! 	estate->err_text = raise_skip_msg;	/* suppress traceback of raise */
  
  	ereport(stmt->elog_level,
  			(err_code ? errcode(err_code) : 0,
--- 2827,2834 
  	/*
  	 * Throw the error (may or may not come back)
  	 */
! 	if (stmt->elog_level < WARNING)
! 		estate->err_text = raise_skip_msg;	/* suppress traceback of raise notice */
  
  	ereport(stmt->elog_level,
  			(err_code ? errcode(err_code) : 0,
*** ./src/test/regress/expected/plpgsql.out.orig	2011-11-24 17:32:30.0 +0100
--- ./src/test/regress/expected/plpgsql.out	2011-11-24 18:26:30.0 +0100
***
*** 1518,1544 
  DETAIL:  Key (name)=(PF1_1) already exists.
  update PSlot set backlink = 'WS.not.there' where slotname = 'PS.base.a1';
  ERROR:  WS.not.there does not exist
! CONTEXT:  PL/pgSQL function "tg_backlink_a()" line 17 at assignment
  update PSlot set backlink = 'XX.illegal' where slotname = 'PS.base.a1';
  ERROR:  illegal backlink beginning with XX
! CONTEXT:  PL/pgSQL function "tg_backlink_a()" line 17 at assignment
  update PSlot set slotlink = 'PS.not.there' where slotname = 'PS.base.a1';
  ERROR:  PS.not.there does not exist
! CONTEXT:  PL/pgSQL function "tg_slotlink_a()" line 17 at assignment
  update PSlot set slotlink = 'XX.illegal' where slotname = 'PS.base.a1';
  ERROR:  illegal slotlink beginning with XX
! CONTEXT:  PL/pgSQL function "tg_slotlink_a()" line 17 at assignment
  insert into HSlot values ('HS', 'base.hub1', 1, '');
  ERROR:  duplicate key value violates unique constraint "hslot_name"
  DETAIL:  Key (slotname)=(HS.base.hub1.1  ) already exists.
  insert into HSlot values ('HS', 'base.hub1', 20, '');
  ERROR:  no manual manipulation of HSlot
  delete from HSlot;
  ERROR:  no manual manipulation of HSlot
  insert into IFace values ('IF', 'notthere', 'eth0', '');
  ERROR:  system "notthere" does not exist
  insert into IFace values ('IF', 'orion', 'ethernet_interface_name_too_long', '');
  ERROR:  IFace slotname "IF.orion.ethernet_interface_name_too_long" too long (20 char max)
  --
  -- The following tests are unrelated to the scenario outlined above;
  -- they merely exercise specific parts of PL/pgSQL
--- 1518,1552 
  DETAIL:  Key (name)=(PF1_1) already exists.
  update PSlot set backlink = 'WS.not.there' where slotname = 'PS.base.a1';
  ERROR:  WS.not.there does not exist
! CONTEXT:  PL/pgSQL function "tg_backlink_set(character,character)" line 30 at RAISE
! PL/pgSQL function "tg_backlink_a()" line 17 at assignment
  update PSlot set backlink = 'XX.illegal' where slotname = 'PS.base.a1';
  ERROR:  illegal backlink beginning with XX
! CONTEXT:  PL/pgSQL function "tg_backlink_set(character,character)" line 47 at RAISE
! PL/pgSQL function "tg_backlink_a()" line 17 at assignment
  update PSlot set slotlink = 'PS.not.there' where slotname = 'PS.base.a1';
  ERROR:  PS.not.there does not exist
! CONTEXT:  PL/pgSQL function "tg_slotlink_set(character,character)" line 30 at RAISE
! PL/pgSQL function "tg_slotlink_a()" line 17 at assignment
  update PSlot set slotlink = 'XX.illegal' where slotname = 'PS.base.a1';
  ERROR:  illegal slotlink beginning with XX
! CONTEXT:  PL/pgSQL function "tg_slotlink_set(character,character)" line 77 at RAISE
! PL/pgSQL function "tg_slotlink_a()" line 17 at assignment
  insert into HSlot values ('HS', 'base.hub1', 1, '');
  ERROR:  duplicate key value violates unique constraint "hslot_name"
  DETAIL:  Key (slotname)=(HS.base.hub1.1  ) already exists.
  insert into HSlot values ('HS', 'base.hub1', 20, '');
  ERROR:  no manual manipulation of HSlot
+ CONTEXT:  PL/pgSQL function "tg_hslot_biu()" line 12 at RAISE
  delete