Re: [HACKERS] PL/pgSQL, RAISE and error context

[Tom Lane]

Joe Conway  writes:
>> On 09/05/2015 09:05 AM, Tom Lane wrote:
>>> Pushed.  I'm not too sure about the expected outputs for python other
>>> than 2.6, nor for sepgsql, but hopefully the buildfarm will provide
>>> feedback.

> One-liner required for sepgsql -- attached committed and pushed.

Thanks for checking!

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] PL/pgSQL, RAISE and error context

[Joe Conway]

On 09/05/2015 09:14 AM, Joe Conway wrote:
> On 09/05/2015 09:05 AM, Tom Lane wrote:
>> I wrote:
>>> If there are not major objections, I'll work on cleaning up and
>>> committing the patch.
>>
>> Pushed.  I'm not too sure about the expected outputs for python other
>> than 2.6, nor for sepgsql, but hopefully the buildfarm will provide
>> feedback.
> 
> We don't have the buildfarm actually checking sepgsql yet, but I'll
> check it out manually today or tomorrow.


One-liner required for sepgsql -- attached committed and pushed.

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development
diff --git a/contrib/sepgsql/expected/label.out b/contrib/sepgsql/expected/label.out
index c84aef7..7af5189 100644
--- a/contrib/sepgsql/expected/label.out
+++ b/contrib/sepgsql/expected/label.out
@@ -156,6 +156,7 @@ LOG:  SELinux: allowed { execute } scontext=unconfined_u:unconfined_r:sepgsql_re
 LOG:  SELinux: allowed { entrypoint } scontext=unconfined_u:unconfined_r:sepgsql_regtest_user_t:s0 tcontext=system_u:object_r:sepgsql_trusted_proc_exec_t:s0 tclass=db_procedure name="function f3()"
 LOG:  SELinux: allowed { transition } scontext=unconfined_u:unconfined_r:sepgsql_regtest_user_t:s0 tcontext=unconfined_u:unconfined_r:sepgsql_trusted_proc_t:s0 tclass=process
 ERROR:  an exception from f3()
+CONTEXT:  PL/pgSQL function f3() line 2 at RAISE
 SELECT f4();			-- failed on domain transition
 LOG:  SELinux: allowed { execute } scontext=unconfined_u:unconfined_r:sepgsql_regtest_user_t:s0 tcontext=system_u:object_r:sepgsql_nosuch_trusted_proc_exec_t:s0 tclass=db_procedure name="public.f4()"
 LOG:  SELinux: allowed { entrypoint } scontext=unconfined_u:unconfined_r:sepgsql_regtest_user_t:s0 tcontext=system_u:object_r:sepgsql_nosuch_trusted_proc_exec_t:s0 tclass=db_procedure name="function f4()"


signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Pavel Stehule]

> Pushed.  I'm not too sure about the expected outputs for python other
> than 2.6, nor for sepgsql, but hopefully the buildfarm will provide
> feedback.
>
>
Thank you very much

Pavel

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Joe Conway]

On 09/05/2015 09:05 AM, Tom Lane wrote:
> I wrote:
>> If there are not major objections, I'll work on cleaning up and
>> committing the patch.
> 
> Pushed.  I'm not too sure about the expected outputs for python other
> than 2.6, nor for sepgsql, but hopefully the buildfarm will provide
> feedback.

We don't have the buildfarm actually checking sepgsql yet, but I'll
check it out manually today or tomorrow.

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Tom Lane]

I wrote:
> If there are not major objections, I'll work on cleaning up and
> committing the patch.

Pushed.  I'm not too sure about the expected outputs for python other
than 2.6, nor for sepgsql, but hopefully the buildfarm will provide
feedback.

BTW, I noticed that the PageOutput line counts for psql's usage(),
slashUsage(), and helpVariables() were all three wrong, which I'm afraid
has been their usual state in the past too.  Since commit 07c8651dd91d5aea
there's been a pretty easy way to check them, which I added comments
about; but I don't hold much hope that that will fix anything.  I wonder
whether there's some way to not need to maintain those counts manually.

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] PL/pgSQL, RAISE and error context

[Tom Lane]

Pavel Stehule  writes:
> 2015-08-12 11:07 GMT+02:00 Marko Tiikkaja :
>> I'm somewhat worried that this is hiding important context from some
>> NOTICE or WARNING messages intended for novice users, but probably not
>> worried enough to go through all of them.  +3/8 from me, I guess.

> I fixed mentioned issues.

Okay, so to summarize where we seem to have ended up:

1. Remove the wart in plpgsql that causes it to suppress the innermost
CONTEXT line for RAISE.  (I think pretty much everyone agrees that this
*is* a wart.  The question is how to get rid of it without a decrease
in usability for simple cases.)

2. Change psql so that by default, it hides the entire CONTEXT output
for messages that are of less than ERROR severity.  Add a new magic
\set variable that allows choosing this behavior, or display CONTEXT
always, or display CONTEXT never.

3. Since psql actually uses libpq for formatting server messages,
add an API to libpq that implements these CONTEXT hide/show options.

The actual code changes are pretty small, but there's rather a large
change in regression test outputs; which is unsurprising, because this
heuristic for what's of interest is entirely different from the old one.

Is everyone satisfied with this solution?  It's okay with me, though
I'm concerned that there will be complaints about loss of backwards
compatibility.  (It's hard to see how the contents of CONTEXT error
fields would be a big application compatibility issue, but you never
know.)

If there are not major objections, I'll work on cleaning up and
committing the patch.  There is still work needed (eg, the API addition
of point 3 is undocumented), but the main question is just whether we
are happy with making things work this way.

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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

Hi

2015-08-12 11:07 GMT+02:00 Marko Tiikkaja :

> On 8/12/15 9:36 AM, Pavel Stehule wrote:
>
>> So, there is common agreement on this version.
>>
>
> There are several instances of double semicolons.  Also,
> PsqlSettings.show_context doesn't look like a boolean to me.  For
> SHOW_CONTEXT, it would be good if the documentation mentioned the default
> value.
>
> I'm somewhat worried that this is hiding important context from some
> NOTICE or WARNING messages intended for novice users, but probably not
> worried enough to go through all of them.  +3/8 from me, I guess.
>

I fixed mentioned issues.

Regards

Pavel

>
>
> .m
>


libpq-context-filter-20150813-01.patch.gz
Description: GNU Zip compressed data

-- 
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-08-12 11:07 GMT+02:00 Marko Tiikkaja :

> On 8/12/15 9:36 AM, Pavel Stehule wrote:
>
>> So, there is common agreement on this version.
>>
>
> There are several instances of double semicolons.  Also,
> PsqlSettings.show_context doesn't look like a boolean to me.  For
> SHOW_CONTEXT, it would be good if the documentation mentioned the default
> value.
>
> I'm somewhat worried that this is hiding important context from some
> NOTICE or WARNING messages intended for novice users, but probably not
> worried enough to go through all of them.  +3/8 from me, I guess.
>
>
Thank you for info

I'll fix it


>
> .m
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Marko Tiikkaja]

On 8/12/15 9:36 AM, Pavel Stehule wrote:

So, there is common agreement on this version.


There are several instances of double semicolons.  Also, 
PsqlSettings.show_context doesn't look like a boolean to me.  For 
SHOW_CONTEXT, it would be good if the documentation mentioned the 
default value.


I'm somewhat worried that this is hiding important context from some 
NOTICE or WARNING messages intended for novice users, but probably not 
worried enough to go through all of them.  +3/8 from me, I guess.



.m


--
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-08-10 18:43 GMT+02:00 Pavel Stehule :

>
>
> 2015-08-10 9:11 GMT+02:00 Heikki Linnakangas :
>
>> On 07/26/2015 08:34 AM, Pavel Stehule wrote:
>>
>>> Hi
>>>
>>> here is complete patch, that introduce context filtering on client side.
>>> The core of this patch is trivial and small - almost all of size are
>>> trivial changes in regress tests - removing useless context.
>>>
>>> Documentation, check-world
>>>
>>
>> Looks good to me at first glance. I'll mark this as Ready for Committer.
>>
>
> Is it acceptable for all?
>
> I have not a problem with this way.
>

So, there is common agreement on this version.

Best regards

Pavel


>
> Regards
>
> Pavel
>
>
>>
>> - Heikki
>>
>>
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-08-10 9:11 GMT+02:00 Heikki Linnakangas :

> On 07/26/2015 08:34 AM, Pavel Stehule wrote:
>
>> Hi
>>
>> here is complete patch, that introduce context filtering on client side.
>> The core of this patch is trivial and small - almost all of size are
>> trivial changes in regress tests - removing useless context.
>>
>> Documentation, check-world
>>
>
> Looks good to me at first glance. I'll mark this as Ready for Committer.
>

Is it acceptable for all?

I have not a problem with this way.

Regards

Pavel


>
> - Heikki
>
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Heikki Linnakangas]

On 07/26/2015 08:34 AM, Pavel Stehule wrote:

Hi

here is complete patch, that introduce context filtering on client side.
The core of this patch is trivial and small - almost all of size are
trivial changes in regress tests - removing useless context.

Documentation, check-world


Looks good to me at first glance. I'll mark this as Ready for Committer.

- Heikki



--
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

Hi

here is complete patch, that introduce context filtering on client side.
The core of this patch is trivial and small - almost all of size are
trivial changes in regress tests - removing useless context.

Documentation, check-world

Regards

Pavel

2015-07-26 0:42 GMT+02:00 Pavel Stehule :

> Hi
>
> I am sending a next variant of filtering context patch.
>
> postgres=# do $$ begin raise notice 'kuku'; end $$;
> NOTICE:  kuku
> DO
> Time: 2.441 ms
> postgres=# do $$ begin raise exception 'kuku'; end $$;
> ERROR:  kuku
> CONTEXT:  PL/pgSQL function inline_code_block line 1 at RAISE
> Time: 0.648 ms
> postgres=# \set SHOW_CONTEXT always
> postgres=# do $$ begin raise notice 'kuku'; end $$;
> NOTICE:  kuku
> CONTEXT:  PL/pgSQL function inline_code_block line 1 at RAISE
> DO
> Time: 0.702 ms
>
> It is a variant, when I try to filter CONTEXT in libpq. There is little
> bit less granularity on libpq side than server side, but still it is enough
> - always, error, none.
>
> This patch is without documentation, but basic regress tests works.
>
> Regards
>
> Pavel
>
>
>
> 2015-07-25 10:01 GMT+02:00 Pavel Stehule :
>
>>
>>
>> 2015-07-21 16:58 GMT+02:00 Merlin Moncure :
>>
>>> On Tue, Jul 21, 2015 at 2:53 AM, Heikki Linnakangas 
>>> wrote:
>>> > On 07/21/2015 10:38 AM, Pavel Stehule wrote:
>>> >>
>>> >> where we are with this patch? Can I do some for it?
>>> >
>>> >
>>> > I still feel this approach is misguided, and we should be tweaking psql
>>> > and/or libpq instead. I don't feel strongly though, and if some other
>>> > committer wants to pick this up in its current form, I won't object.
>>> So this
>>> > patch has reached an impasse, and if no-one else wants to pick this
>>> up, I'm
>>> > going to mark this as "Returned with Feedback" and move on.
>>>
>>> That's unfortunate.  Maybe I'm missing something:
>>>
>>> What does a client side implementation offer that a server side
>>> implementation does not offer?
>>>
>>
>> I have not any problem to change the filtering to client side. Primary
>> question is fix of PLpgSQL RAISE statement issue - The context field
>> filtering is a necessary follow-up and trivial in both cases.
>>
>> In this case, it is acceptable for all?
>>
>> Regards
>>
>> Pavel
>>
>>
>>>
>>> merlin
>>>
>>
>>
>


libpq-context-filter-20150726-01.patch.gz
Description: GNU Zip compressed data

-- 
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

Hi

I am sending a next variant of filtering context patch.

postgres=# do $$ begin raise notice 'kuku'; end $$;
NOTICE:  kuku
DO
Time: 2.441 ms
postgres=# do $$ begin raise exception 'kuku'; end $$;
ERROR:  kuku
CONTEXT:  PL/pgSQL function inline_code_block line 1 at RAISE
Time: 0.648 ms
postgres=# \set SHOW_CONTEXT always
postgres=# do $$ begin raise notice 'kuku'; end $$;
NOTICE:  kuku
CONTEXT:  PL/pgSQL function inline_code_block line 1 at RAISE
DO
Time: 0.702 ms

It is a variant, when I try to filter CONTEXT in libpq. There is little bit
less granularity on libpq side than server side, but still it is enough -
always, error, none.

This patch is without documentation, but basic regress tests works.

Regards

Pavel



2015-07-25 10:01 GMT+02:00 Pavel Stehule :

>
>
> 2015-07-21 16:58 GMT+02:00 Merlin Moncure :
>
>> On Tue, Jul 21, 2015 at 2:53 AM, Heikki Linnakangas 
>> wrote:
>> > On 07/21/2015 10:38 AM, Pavel Stehule wrote:
>> >>
>> >> where we are with this patch? Can I do some for it?
>> >
>> >
>> > I still feel this approach is misguided, and we should be tweaking psql
>> > and/or libpq instead. I don't feel strongly though, and if some other
>> > committer wants to pick this up in its current form, I won't object. So
>> this
>> > patch has reached an impasse, and if no-one else wants to pick this up,
>> I'm
>> > going to mark this as "Returned with Feedback" and move on.
>>
>> That's unfortunate.  Maybe I'm missing something:
>>
>> What does a client side implementation offer that a server side
>> implementation does not offer?
>>
>
> I have not any problem to change the filtering to client side. Primary
> question is fix of PLpgSQL RAISE statement issue - The context field
> filtering is a necessary follow-up and trivial in both cases.
>
> In this case, it is acceptable for all?
>
> Regards
>
> Pavel
>
>
>>
>> merlin
>>
>
>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
new file mode 100644
index 6181a61..7168809
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*** SyncVariables(void)
*** 2029,2034 
--- 2029,2035 
  
  	/* send stuff to it, too */
  	PQsetErrorVerbosity(pset.db, pset.verbosity);
+ 	PQsetErrorContextVisibility(pset.db, pset.show_context);
  }
  
  /*
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
new file mode 100644
index d3e3114..0bc97de
*** a/src/bin/psql/help.c
--- b/src/bin/psql/help.c
*** helpVariables(unsigned short int pager)
*** 307,313 
  {
  	FILE	   *output;
  
! 	output = PageOutput(85, pager ? &(pset.popt.topt) : NULL);
  
  	fprintf(output, _("List of specially treated variables.\n"));
  
--- 307,313 
  {
  	FILE	   *output;
  
! 	output = PageOutput(86, pager ? &(pset.popt.topt) : NULL);
  
  	fprintf(output, _("List of specially treated variables.\n"));
  
*** helpVariables(unsigned short int pager)
*** 339,344 
--- 339,345 
  	fprintf(output, _("  PROMPT2specify the prompt used when a statement continues from a previous line\n"));
  	fprintf(output, _("  PROMPT3specify the prompt used during COPY ... FROM STDIN\n"));
  	fprintf(output, _("  QUIET  run quietly (same as -q option)\n"));
+ 	fprintf(output, _("  SHOW_CONTEXT   when a error context will be displayed [always, error, none]\n"));
  	fprintf(output, _("  SINGLELINE end of line terminates SQL command mode (same as -S option)\n"));
  	fprintf(output, _("  SINGLESTEP single-step mode (same as -s option)\n"));
  	fprintf(output, _("  USER   the currently connected database user\n"));
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
new file mode 100644
index d34dc28..5b49059
*** a/src/bin/psql/settings.h
--- b/src/bin/psql/settings.h
*** typedef struct _psqlSettings
*** 129,134 
--- 129,135 
  	const char *prompt2;
  	const char *prompt3;
  	PGVerbosity verbosity;		/* current error verbosity level */
+ 	bool		show_context;
  } PsqlSettings;
  
  extern PsqlSettings pset;
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
new file mode 100644
index 28ba75a..534c914
*** a/src/bin/psql/startup.c
--- b/src/bin/psql/startup.c
*** main(int argc, char *argv[])
*** 157,162 
--- 157,163 
  	SetVariable(pset.vars, "PROMPT1", DEFAULT_PROMPT1);
  	SetVariable(pset.vars, "PROMPT2", DEFAULT_PROMPT2);
  	SetVariable(pset.vars, "PROMPT3", DEFAULT_PROMPT3);
+ 	SetVariable(pset.vars, "SHOW_CONTEXT", "error");;
  
  	parse_psql_options(argc, argv, &options);
  
*** verbosity_hook(const char *newval)
*** 868,873 
--- 869,895 
  		PQsetErrorVerbosity(pset.db, pset.verbosity);
  }
  
+ static void
+ show_context_hook(const char *newval)
+ {
+ 	if (newval == NULL)
+ 		pset.show_context = PQSHOW_CONTEXT_ERROR;
+ 	else if (pg_strcasecmp(newval, "always") == 0)
+ 		pset.show_context = PQSHOW_CONTEXT_ALL;
+ 	else if (pg_strcasecmp(newval, "error") == 0)
+ 		pset.show_context = PQSHOW_CONTEXT_ERRO

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-07-21 16:58 GMT+02:00 Merlin Moncure :

> On Tue, Jul 21, 2015 at 2:53 AM, Heikki Linnakangas 
> wrote:
> > On 07/21/2015 10:38 AM, Pavel Stehule wrote:
> >>
> >> where we are with this patch? Can I do some for it?
> >
> >
> > I still feel this approach is misguided, and we should be tweaking psql
> > and/or libpq instead. I don't feel strongly though, and if some other
> > committer wants to pick this up in its current form, I won't object. So
> this
> > patch has reached an impasse, and if no-one else wants to pick this up,
> I'm
> > going to mark this as "Returned with Feedback" and move on.
>
> That's unfortunate.  Maybe I'm missing something:
>
> What does a client side implementation offer that a server side
> implementation does not offer?
>

I have not any problem to change the filtering to client side. Primary
question is fix of PLpgSQL RAISE statement issue - The context field
filtering is a necessary follow-up and trivial in both cases.

In this case, it is acceptable for all?

Regards

Pavel


>
> merlin
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Merlin Moncure]

On Tue, Jul 21, 2015 at 2:53 AM, Heikki Linnakangas  wrote:
> On 07/21/2015 10:38 AM, Pavel Stehule wrote:
>>
>> where we are with this patch? Can I do some for it?
>
>
> I still feel this approach is misguided, and we should be tweaking psql
> and/or libpq instead. I don't feel strongly though, and if some other
> committer wants to pick this up in its current form, I won't object. So this
> patch has reached an impasse, and if no-one else wants to pick this up, I'm
> going to mark this as "Returned with Feedback" and move on.

That's unfortunate.  Maybe I'm missing something:

What does a client side implementation offer that a server side
implementation does not offer?

merlin


-- 
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-07-21 9:53 GMT+02:00 Heikki Linnakangas :

> On 07/21/2015 10:38 AM, Pavel Stehule wrote:
>
>> where we are with this patch? Can I do some for it?
>>
>
> I still feel this approach is misguided, and we should be tweaking psql
> and/or libpq instead. I don't feel strongly though, and if some other
> committer wants to pick this up in its current form, I won't object. So
> this patch has reached an impasse, and if no-one else wants to pick this
> up, I'm going to mark this as "Returned with Feedback" and move on.
>

Can we define, when we have a agreement and where not? The missing context
for RAISE EXCEPTION statement is a important issue and I would to solve it.

last patch has two parts:

1. remove plpgsql fix, that remove context for plpgsql RAISE statement - it
is working good enough for less NOTICE level, and work badly for EXCEPTION
and higher level.

2. enforce filtering of CONTEXT field on both sides (client/log)

For me, @1 is important and good solution (because there is strange
inconsistency between PLpgSQL and any other PL), @2 allows more ways - but
probably log_min_context (WARNING) is good idea too.

The advantage of context filtering on server side (client_min_message) is
one - it can be controlled by plpgsql - so I can do dynamic decision if
some NOTICE will have context or not.

The complexity will be +/- same for psql/libpq or for server side filtering.

Regards

Pavel


> - Heikki
>
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Heikki Linnakangas]

On 07/21/2015 10:38 AM, Pavel Stehule wrote:

where we are with this patch? Can I do some for it?


I still feel this approach is misguided, and we should be tweaking psql 
and/or libpq instead. I don't feel strongly though, and if some other 
committer wants to pick this up in its current form, I won't object. So 
this patch has reached an impasse, and if no-one else wants to pick this 
up, I'm going to mark this as "Returned with Feedback" and move on.

- Heikki



--
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-07-09 23:16 GMT+02:00 Pavel Stehule :

>
>
> 2015-07-09 22:57 GMT+02:00 Merlin Moncure :
>
>> On Thu, Jul 9, 2015 at 3:31 PM, Pavel Stehule 
>> wrote:
>> >
>> >
>> > 2015-07-09 20:08 GMT+02:00 Merlin Moncure :
>> >>
>> >> On Thu, Jul 9, 2015 at 10:48 AM, Pavel Stehule <
>> pavel.steh...@gmail.com>
>> >> wrote:
>> >> >
>> >> > 2015-07-09 15:17 GMT+02:00 Merlin Moncure :
>> >> >>
>> >> >> On Wed, Jul 8, 2015 at 11:28 PM, Pavel Stehule
>> >> >> 
>>
>> >> >> wrote:
>> >> >> > Hi
>> >> >> >
>> >> >> > second version of this patch
>> >> >> >
>> >> >> > make check-world passed
>> >> >>
>> >> >> quickly scanning the patch, the implementation is trivial (minus
>> >> >> regression test adjustments), and is, IMSNSHO, the right solution.
>> >> >
>> >> >
>> >> > yes, it is right way - the behave of RAISE statement will be much
>> more
>> >> > cleaner
>> >> >>
>> >> >>
>> >> >> Several of the source level comments need some minor wordsmithing
>> and
>> >> >> the GUCs are missing documentation.  If we've got consensus on the
>> >> >> approach, I'll pitch in on that.
>> >> >
>> >> > thank you
>> >>
>> >> revised patch attached. added GUC docs and cleaned up pg_settings
>> >> language.  Also tested patch and it works beautifully.
>> >>
>> >> Note, Pavel's patch does adjust default behavior to what we think is
>> >> the "right" settings.
>> >
>> >
>> > Thank you for documentation.
>> >
>> > There is small error - default for client_min_context is error - not
>> notice.
>> > With this level a diff from regress tests is minimal. Default for
>> > log_min_context should be warning.
>>
>> whoop!  thanks.   Also, I was playing a bit with the idea of making
>> client_min_context "superuser only" setting.  The idea being this
>> could be used to prevent leakage of stored procedure code in cases
>> where the admins don't want it to be exposed.  I figured it was a bad
>> idea though; it would frustrate debugging in reasonable cases.
>>
>
> This is not designed for security usage. Probably there can be some rule
> in future - the possibility to see or don't see  a error context - OFF, ON.
> For this reason, the setting a some min level is not good way.
>

Hi

where we are with this patch? Can I do some for it?

Regards

Pavel


>
> Regards
>
> Pavel
>
>
>
>
>>
>> merlin
>>
>
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-07-09 22:57 GMT+02:00 Merlin Moncure :

> On Thu, Jul 9, 2015 at 3:31 PM, Pavel Stehule 
> wrote:
> >
> >
> > 2015-07-09 20:08 GMT+02:00 Merlin Moncure :
> >>
> >> On Thu, Jul 9, 2015 at 10:48 AM, Pavel Stehule  >
> >> wrote:
> >> >
> >> > 2015-07-09 15:17 GMT+02:00 Merlin Moncure :
> >> >>
> >> >> On Wed, Jul 8, 2015 at 11:28 PM, Pavel Stehule
> >> >> 
> >> >> wrote:
> >> >> > Hi
> >> >> >
> >> >> > second version of this patch
> >> >> >
> >> >> > make check-world passed
> >> >>
> >> >> quickly scanning the patch, the implementation is trivial (minus
> >> >> regression test adjustments), and is, IMSNSHO, the right solution.
> >> >
> >> >
> >> > yes, it is right way - the behave of RAISE statement will be much more
> >> > cleaner
> >> >>
> >> >>
> >> >> Several of the source level comments need some minor wordsmithing and
> >> >> the GUCs are missing documentation.  If we've got consensus on the
> >> >> approach, I'll pitch in on that.
> >> >
> >> > thank you
> >>
> >> revised patch attached. added GUC docs and cleaned up pg_settings
> >> language.  Also tested patch and it works beautifully.
> >>
> >> Note, Pavel's patch does adjust default behavior to what we think is
> >> the "right" settings.
> >
> >
> > Thank you for documentation.
> >
> > There is small error - default for client_min_context is error - not
> notice.
> > With this level a diff from regress tests is minimal. Default for
> > log_min_context should be warning.
>
> whoop!  thanks.   Also, I was playing a bit with the idea of making
> client_min_context "superuser only" setting.  The idea being this
> could be used to prevent leakage of stored procedure code in cases
> where the admins don't want it to be exposed.  I figured it was a bad
> idea though; it would frustrate debugging in reasonable cases.
>

This is not designed for security usage. Probably there can be some rule in
future - the possibility to see or don't see  a error context - OFF, ON.
For this reason, the setting a some min level is not good way.

Regards

Pavel




>
> merlin
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Merlin Moncure]

On Thu, Jul 9, 2015 at 3:31 PM, Pavel Stehule  wrote:
>
>
> 2015-07-09 20:08 GMT+02:00 Merlin Moncure :
>>
>> On Thu, Jul 9, 2015 at 10:48 AM, Pavel Stehule 
>> wrote:
>> >
>> > 2015-07-09 15:17 GMT+02:00 Merlin Moncure :
>> >>
>> >> On Wed, Jul 8, 2015 at 11:28 PM, Pavel Stehule
>> >> 
>> >> wrote:
>> >> > Hi
>> >> >
>> >> > second version of this patch
>> >> >
>> >> > make check-world passed
>> >>
>> >> quickly scanning the patch, the implementation is trivial (minus
>> >> regression test adjustments), and is, IMSNSHO, the right solution.
>> >
>> >
>> > yes, it is right way - the behave of RAISE statement will be much more
>> > cleaner
>> >>
>> >>
>> >> Several of the source level comments need some minor wordsmithing and
>> >> the GUCs are missing documentation.  If we've got consensus on the
>> >> approach, I'll pitch in on that.
>> >
>> > thank you
>>
>> revised patch attached. added GUC docs and cleaned up pg_settings
>> language.  Also tested patch and it works beautifully.
>>
>> Note, Pavel's patch does adjust default behavior to what we think is
>> the "right" settings.
>
>
> Thank you for documentation.
>
> There is small error - default for client_min_context is error - not notice.
> With this level a diff from regress tests is minimal. Default for
> log_min_context should be warning.

whoop!  thanks.   Also, I was playing a bit with the idea of making
client_min_context "superuser only" setting.  The idea being this
could be used to prevent leakage of stored procedure code in cases
where the admins don't want it to be exposed.  I figured it was a bad
idea though; it would frustrate debugging in reasonable cases.

merlin


-- 
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-07-09 20:08 GMT+02:00 Merlin Moncure :

> On Thu, Jul 9, 2015 at 10:48 AM, Pavel Stehule 
> wrote:
> >
> > 2015-07-09 15:17 GMT+02:00 Merlin Moncure :
> >>
> >> On Wed, Jul 8, 2015 at 11:28 PM, Pavel Stehule  >
> >> wrote:
> >> > Hi
> >> >
> >> > second version of this patch
> >> >
> >> > make check-world passed
> >>
> >> quickly scanning the patch, the implementation is trivial (minus
> >> regression test adjustments), and is, IMSNSHO, the right solution.
> >
> >
> > yes, it is right way - the behave of RAISE statement will be much more
> > cleaner
> >>
> >>
> >> Several of the source level comments need some minor wordsmithing and
> >> the GUCs are missing documentation.  If we've got consensus on the
> >> approach, I'll pitch in on that.
> >
> > thank you
>
> revised patch attached. added GUC docs and cleaned up pg_settings
> language.  Also tested patch and it works beautifully.
>
> Note, Pavel's patch does adjust default behavior to what we think is
> the "right" settings.
>

Thank you for documentation.

There is small error - default for client_min_context is error - not
notice. With this level a diff from regress tests is minimal. Default for
log_min_context should be warning.

 Regards

Pavel


>
> merlin
>
diff --git a/contrib/dblink/expected/dblink.out b/contrib/dblink/expected/dblink.out
new file mode 100644
index a49b562..a268fc7
*** a/contrib/dblink/expected/dblink.out
--- b/contrib/dblink/expected/dblink.out
***
*** 1,3 
--- 1,4 
+ set client_min_context TO notice;
  CREATE EXTENSION dblink;
  CREATE TABLE foo(f1 int, f2 text, f3 text[], primary key (f1,f2));
  INSERT INTO foo VALUES (0,'a','{"a0","b0","c0"}');
diff --git a/contrib/dblink/sql/dblink.sql b/contrib/dblink/sql/dblink.sql
new file mode 100644
index ea78cc2..cf7e57e
*** a/contrib/dblink/sql/dblink.sql
--- b/contrib/dblink/sql/dblink.sql
***
*** 1,3 
--- 1,5 
+ set client_min_context TO notice;
+ 
  CREATE EXTENSION dblink;
  
  CREATE TABLE foo(f1 int, f2 text, f3 text[], primary key (f1,f2));
diff --git a/contrib/hstore_plperl/expected/hstore_plperlu.out b/contrib/hstore_plperl/expected/hstore_plperlu.out
new file mode 100644
index 8c689ad..c97fd3f
*** a/contrib/hstore_plperl/expected/hstore_plperlu.out
--- b/contrib/hstore_plperl/expected/hstore_plperlu.out
*** INFO:  $VAR1 = {
*** 29,35 
'cc' => undef
  };
  
- CONTEXT:  PL/Perl function "test1"
   test1 
  ---
   2
--- 29,34 
*** $$;
*** 46,52 
  SELECT test1none('aa=>bb, cc=>NULL'::hstore);
  INFO:  $VAR1 = '"aa"=>"bb", "cc"=>NULL';
  
- CONTEXT:  PL/Perl function "test1none"
   test1none 
  ---
   0
--- 45,50 
*** INFO:  $VAR1 = {
*** 67,73 
'cc' => undef
  };
  
- CONTEXT:  PL/Perl function "test1list"
   test1list 
  ---
   2
--- 65,70 
*** $VAR2 = {
*** 92,98 
'dd' => 'ee'
  };
  
- CONTEXT:  PL/Perl function "test1arr"
   test1arr 
  --
  2
--- 89,94 
*** INFO:  $VAR1 = {
*** 120,129 
'cc' => undef
  };
  
- CONTEXT:  PL/Perl function "test3"
  INFO:  $VAR1 = '"a"=>"1", "b"=>"boo", "c"=>NULL';
  
- CONTEXT:  PL/Perl function "test3"
   test3 
  ---
   
--- 116,123 
*** INFO:  $VAR1 = {
*** 161,167 
 }
  };
  
- CONTEXT:  PL/Perl function "test4"
  SELECT * FROM test1;
   a |b
  ---+-
--- 155,160 
diff --git a/contrib/hstore_plpython/expected/hstore_plpython.out b/contrib/hstore_plpython/expected/hstore_plpython.out
new file mode 100644
index b7a6a92..23091d3
*** a/contrib/hstore_plpython/expected/hstore_plpython.out
--- b/contrib/hstore_plpython/expected/hstore_plpython.out
*** return len(val)
*** 13,19 
  $$;
  SELECT test1('aa=>bb, cc=>NULL'::hstore);
  INFO:  [('aa', 'bb'), ('cc', None)]
- CONTEXT:  PL/Python function "test1"
   test1 
  ---
   2
--- 13,18 
*** return len(val)
*** 32,38 
  $$;
  SELECT test1n('aa=>bb, cc=>NULL'::hstore);
  INFO:  [('aa', 'bb'), ('cc', None)]
- CONTEXT:  PL/Python function "test1n"
   test1n 
  
2
--- 31,36 
diff --git a/contrib/ltree_plpython/expected/ltree_plpython.out b/contrib/ltree_plpython/expected/ltree_plpython.out
new file mode 100644
index 934529e..c6e8a7c
*** a/contrib/ltree_plpython/expected/ltree_plpython.out
--- b/contrib/ltree_plpython/expected/ltree_plpython.out
*** return len(val)
*** 9,15 
  $$;
  SELECT test1('aa.bb.cc'::ltree);
  INFO:  ['aa', 'bb', 'cc']
- CONTEXT:  PL/Python function "test1"
   test1 
  ---
   3
--- 9,14 
*** return len(val)
*** 24,30 
  $$;
  SELECT test1n('aa.bb.cc'::ltree);
  INFO:  ['aa', 'bb', 'cc']
- CONTEXT:  PL/Python function "test1n"
   test1n 
  
3
--- 23,28 
diff --git a/do

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Merlin Moncure]

On Thu, Jul 9, 2015 at 10:48 AM, Pavel Stehule  wrote:
>
> 2015-07-09 15:17 GMT+02:00 Merlin Moncure :
>>
>> On Wed, Jul 8, 2015 at 11:28 PM, Pavel Stehule 
>> wrote:
>> > Hi
>> >
>> > second version of this patch
>> >
>> > make check-world passed
>>
>> quickly scanning the patch, the implementation is trivial (minus
>> regression test adjustments), and is, IMSNSHO, the right solution.
>
>
> yes, it is right way - the behave of RAISE statement will be much more
> cleaner
>>
>>
>> Several of the source level comments need some minor wordsmithing and
>> the GUCs are missing documentation.  If we've got consensus on the
>> approach, I'll pitch in on that.
>
> thank you

revised patch attached. added GUC docs and cleaned up pg_settings
language.  Also tested patch and it works beautifully.

Note, Pavel's patch does adjust default behavior to what we think is
the "right" settings.

merlin
diff --git a/contrib/dblink/expected/dblink.out b/contrib/dblink/expected/dblink.out
new file mode 100644
index a49b562..a268fc7
*** a/contrib/dblink/expected/dblink.out
--- b/contrib/dblink/expected/dblink.out
***
*** 1,3 
--- 1,4 
+ set client_min_context TO notice;
  CREATE EXTENSION dblink;
  CREATE TABLE foo(f1 int, f2 text, f3 text[], primary key (f1,f2));
  INSERT INTO foo VALUES (0,'a','{"a0","b0","c0"}');
diff --git a/contrib/dblink/sql/dblink.sql b/contrib/dblink/sql/dblink.sql
new file mode 100644
index ea78cc2..cf7e57e
*** a/contrib/dblink/sql/dblink.sql
--- b/contrib/dblink/sql/dblink.sql
***
*** 1,3 
--- 1,5 
+ set client_min_context TO notice;
+ 
  CREATE EXTENSION dblink;
  
  CREATE TABLE foo(f1 int, f2 text, f3 text[], primary key (f1,f2));
diff --git a/contrib/hstore_plperl/expected/hstore_plperlu.out b/contrib/hstore_plperl/expected/hstore_plperlu.out
new file mode 100644
index 8c689ad..c97fd3f
*** a/contrib/hstore_plperl/expected/hstore_plperlu.out
--- b/contrib/hstore_plperl/expected/hstore_plperlu.out
*** INFO:  $VAR1 = {
*** 29,35 
'cc' => undef
  };
  
- CONTEXT:  PL/Perl function "test1"
   test1 
  ---
   2
--- 29,34 
*** $$;
*** 46,52 
  SELECT test1none('aa=>bb, cc=>NULL'::hstore);
  INFO:  $VAR1 = '"aa"=>"bb", "cc"=>NULL';
  
- CONTEXT:  PL/Perl function "test1none"
   test1none 
  ---
   0
--- 45,50 
*** INFO:  $VAR1 = {
*** 67,73 
'cc' => undef
  };
  
- CONTEXT:  PL/Perl function "test1list"
   test1list 
  ---
   2
--- 65,70 
*** $VAR2 = {
*** 92,98 
'dd' => 'ee'
  };
  
- CONTEXT:  PL/Perl function "test1arr"
   test1arr 
  --
  2
--- 89,94 
*** INFO:  $VAR1 = {
*** 120,129 
'cc' => undef
  };
  
- CONTEXT:  PL/Perl function "test3"
  INFO:  $VAR1 = '"a"=>"1", "b"=>"boo", "c"=>NULL';
  
- CONTEXT:  PL/Perl function "test3"
   test3 
  ---
   
--- 116,123 
*** INFO:  $VAR1 = {
*** 161,167 
 }
  };
  
- CONTEXT:  PL/Perl function "test4"
  SELECT * FROM test1;
   a |b
  ---+-
--- 155,160 
diff --git a/contrib/hstore_plpython/expected/hstore_plpython.out b/contrib/hstore_plpython/expected/hstore_plpython.out
new file mode 100644
index b7a6a92..23091d3
*** a/contrib/hstore_plpython/expected/hstore_plpython.out
--- b/contrib/hstore_plpython/expected/hstore_plpython.out
*** return len(val)
*** 13,19 
  $$;
  SELECT test1('aa=>bb, cc=>NULL'::hstore);
  INFO:  [('aa', 'bb'), ('cc', None)]
- CONTEXT:  PL/Python function "test1"
   test1 
  ---
   2
--- 13,18 
*** return len(val)
*** 32,38 
  $$;
  SELECT test1n('aa=>bb, cc=>NULL'::hstore);
  INFO:  [('aa', 'bb'), ('cc', None)]
- CONTEXT:  PL/Python function "test1n"
   test1n 
  
2
--- 31,36 
diff --git a/contrib/ltree_plpython/expected/ltree_plpython.out b/contrib/ltree_plpython/expected/ltree_plpython.out
new file mode 100644
index 934529e..c6e8a7c
*** a/contrib/ltree_plpython/expected/ltree_plpython.out
--- b/contrib/ltree_plpython/expected/ltree_plpython.out
*** return len(val)
*** 9,15 
  $$;
  SELECT test1('aa.bb.cc'::ltree);
  INFO:  ['aa', 'bb', 'cc']
- CONTEXT:  PL/Python function "test1"
   test1 
  ---
   3
--- 9,14 
*** return len(val)
*** 24,30 
  $$;
  SELECT test1n('aa.bb.cc'::ltree);
  INFO:  ['aa', 'bb', 'cc']
- CONTEXT:  PL/Python function "test1n"
   test1n 
  
3
--- 23,28 
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
new file mode 100644
index b91d6c7..38ae0ad
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
*** local0.*/var/log/postgresql
*** 4144,4149 
--- 4144,4171 
  
   
  
+  
+   client_min_context (enum)
+   
+client_min_context configura

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-07-09 15:17 GMT+02:00 Merlin Moncure :

> On Wed, Jul 8, 2015 at 11:28 PM, Pavel Stehule 
> wrote:
> > Hi
> >
> > second version of this patch
> >
> > make check-world passed
>
> quickly scanning the patch, the implementation is trivial (minus
> regression test adjustments), and is, IMSNSHO, the right solution.
>

yes, it is right way - the behave of RAISE statement will be much more
cleaner

>
> Several of the source level comments need some minor wordsmithing and
> the GUCs are missing documentation.  If we've got consensus on the
> approach, I'll pitch in on that.
>

thank you

Pavel


>
> merlin
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Merlin Moncure]

On Wed, Jul 8, 2015 at 11:28 PM, Pavel Stehule  wrote:
> Hi
>
> second version of this patch
>
> make check-world passed

quickly scanning the patch, the implementation is trivial (minus
regression test adjustments), and is, IMSNSHO, the right solution.

Several of the source level comments need some minor wordsmithing and
the GUCs are missing documentation.  If we've got consensus on the
approach, I'll pitch in on that.

merlin


-- 
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-07-08 23:46 GMT+02:00 Merlin Moncure :

> On Wed, Jul 8, 2015 at 4:39 PM, Pavel Stehule 
> wrote:
> > 2015-07-08 8:35 GMT+02:00 Pavel Stehule :
> >>
> >>
> >>
> >> 2015-07-07 18:15 GMT+02:00 Merlin Moncure :
> >>>
> >>> On Tue, Jul 7, 2015 at 9:04 AM, Pavel Stehule  >
> >>> wrote:
> >>> >> It doesn't have to if the behavior is guarded with a GUC.  I just
> >>> >> don't understand what all the fuss is about.  The default behavior
> of
> >>> >> logging that is well established by other languages (for example
> java)
> >>> >> that manage error stack for you should be to:
> >>> >>
> >>> >> *) Give stack trace when an uncaught exception is thrown
> >>> >> *) Do not give stack trace in all other logging cases unless asked
> for
> >>> >
> >>> > what is RAISE EXCEPTION - first or second case?
> >>>
> >>> First: RAISE (unless caught) is no different than any other kind of
> >>> error.
> >>>
> >>> On Tue, Jul 7, 2015 at 9:42 AM, Heikki Linnakangas 
> >>> wrote:
> >>> > On 07/07/2015 04:56 PM, Merlin Moncure wrote:
> >>> >> It doesn't have to if the behavior is guarded with a GUC.  I just
> >>> >> don't understand what all the fuss is about.  The default behavior
> of
> >>> >> logging that is well established by other languages (for example
> java)
> >>> >> that manage error stack for you should be to:
> >>> >>
> >>> >> *) Give stack trace when an uncaught exception is thrown
> >>> >> *) Do not give stack trace in all other logging cases unless asked
> for
> >>> >
> >>> > Java's exception handling is so different from PostgreSQL's errors
> that
> >>> > I
> >>> > don't think there's much to be learned from that. But I'll bite:
> >>> >
> >>> > First of all, Java's exceptions always contain a stack trace. It's up
> >>> > to you
> >>> > when you catch an exception to decide whether to print it or not.
> "try
> >>> > { ...
> >>> > } catch (Exception e) { e.printStackTrace() }" is fairly common,
> >>> > actually.
> >>> > There is nothing like a NOTICE in Java, i.e. an exception that's
> thrown
> >>> > but
> >>> > doesn't affect the control flow. The best I can think of is
> >>> > System.out.println(), which of course has no stack trace attached to
> >>> > it.
> >>>
> >>> exactly.
> >>>
> >>> > Perhaps you're arguing that NOTICE is more like printing to stderr,
> and
> >>> > should never contain any context information. I don't think that
> would
> >>> > be an
> >>> > improvement. It's very handy to have the context information
> available
> >>> > if
> >>> > don't know where a NOTICE is coming from, even if in most cases
> you're
> >>> > not
> >>> > interested in it.
> >>>
> >>> That's exactly what I'm arguing.  NOTICE (and WARNING) are for
> >>> printing out information to client side logging; it's really the only
> >>> tool we have for that purpose and it fits that role perfectly.  Of
> >>> course, you may want to have NOTICE print context, especially when
> >>> debugging, but some control over that would be nice and in most cases
> >>> it's really not necessary.  I really don't understand the objection to
> >>> offering control over that behavior although I certainly understand
> >>> wanting to keep the default behavior as it currently is.
> >>>
> >>> > This is really quite different from a programming language's
> exception
> >>> > handling. First, there's a server, which produces the errors, and a
> >>> > separate
> >>> > client, which displays them. You cannot catch an exception in the
> >>> > client.
> >>> >
> >>> > BTW, let me throw in one use case to consider. We've been talking
> about
> >>> > psql, and what to print, but imagine a more sophisticated client like
> >>> > pgAdmin. It's not limited to either printing the context or not. It
> >>> > could
> >>> > e.g. hide the context information of all messages when they occur,
> but
> >>> > if
> >>> > you double-click on it, it's expanded to show all the context,
> location
> >>> > and
> >>> > all. You can't do that if the server doesn't send the context
> >>> > information in
> >>> > the first place.
> >>> >
> >>> >> I would be happy to show you the psql redirected output logs from my
> >>> >> nightly server processes that spew into the megabytes because of
> >>> >> logging various high level steps (did this, did that).
> >>> >
> >>> > Oh, I believe you. I understand what the problem is, we're only
> talking
> >>> > about how best to address it.
> >>>
> >>> Yeah.  For posterity, a psql based solution would work fine for me,
> >>> but a server side solution has a lot of advantages (less protocol
> >>> chatter, more configurability, keeping libpq/psql light).
> >>
> >>
> >> After some work on reduced version of "plpgsql.min_context" patch I am
> >> inclining to think so ideal solution needs more steps - because this
> issue
> >> has more than one dimension.
> >>
> >> There are two independent issues:
> >>
> >> 1. old plpgsql workaround that reduced the unwanted call stack info for
> >> RAISE NOTICE. Negative side effect of this workaround is missing context
> >> 

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Merlin Moncure]

On Wed, Jul 8, 2015 at 4:39 PM, Pavel Stehule  wrote:
> 2015-07-08 8:35 GMT+02:00 Pavel Stehule :
>>
>>
>>
>> 2015-07-07 18:15 GMT+02:00 Merlin Moncure :
>>>
>>> On Tue, Jul 7, 2015 at 9:04 AM, Pavel Stehule 
>>> wrote:
>>> >> It doesn't have to if the behavior is guarded with a GUC.  I just
>>> >> don't understand what all the fuss is about.  The default behavior of
>>> >> logging that is well established by other languages (for example java)
>>> >> that manage error stack for you should be to:
>>> >>
>>> >> *) Give stack trace when an uncaught exception is thrown
>>> >> *) Do not give stack trace in all other logging cases unless asked for
>>> >
>>> > what is RAISE EXCEPTION - first or second case?
>>>
>>> First: RAISE (unless caught) is no different than any other kind of
>>> error.
>>>
>>> On Tue, Jul 7, 2015 at 9:42 AM, Heikki Linnakangas 
>>> wrote:
>>> > On 07/07/2015 04:56 PM, Merlin Moncure wrote:
>>> >> It doesn't have to if the behavior is guarded with a GUC.  I just
>>> >> don't understand what all the fuss is about.  The default behavior of
>>> >> logging that is well established by other languages (for example java)
>>> >> that manage error stack for you should be to:
>>> >>
>>> >> *) Give stack trace when an uncaught exception is thrown
>>> >> *) Do not give stack trace in all other logging cases unless asked for
>>> >
>>> > Java's exception handling is so different from PostgreSQL's errors that
>>> > I
>>> > don't think there's much to be learned from that. But I'll bite:
>>> >
>>> > First of all, Java's exceptions always contain a stack trace. It's up
>>> > to you
>>> > when you catch an exception to decide whether to print it or not. "try
>>> > { ...
>>> > } catch (Exception e) { e.printStackTrace() }" is fairly common,
>>> > actually.
>>> > There is nothing like a NOTICE in Java, i.e. an exception that's thrown
>>> > but
>>> > doesn't affect the control flow. The best I can think of is
>>> > System.out.println(), which of course has no stack trace attached to
>>> > it.
>>>
>>> exactly.
>>>
>>> > Perhaps you're arguing that NOTICE is more like printing to stderr, and
>>> > should never contain any context information. I don't think that would
>>> > be an
>>> > improvement. It's very handy to have the context information available
>>> > if
>>> > don't know where a NOTICE is coming from, even if in most cases you're
>>> > not
>>> > interested in it.
>>>
>>> That's exactly what I'm arguing.  NOTICE (and WARNING) are for
>>> printing out information to client side logging; it's really the only
>>> tool we have for that purpose and it fits that role perfectly.  Of
>>> course, you may want to have NOTICE print context, especially when
>>> debugging, but some control over that would be nice and in most cases
>>> it's really not necessary.  I really don't understand the objection to
>>> offering control over that behavior although I certainly understand
>>> wanting to keep the default behavior as it currently is.
>>>
>>> > This is really quite different from a programming language's exception
>>> > handling. First, there's a server, which produces the errors, and a
>>> > separate
>>> > client, which displays them. You cannot catch an exception in the
>>> > client.
>>> >
>>> > BTW, let me throw in one use case to consider. We've been talking about
>>> > psql, and what to print, but imagine a more sophisticated client like
>>> > pgAdmin. It's not limited to either printing the context or not. It
>>> > could
>>> > e.g. hide the context information of all messages when they occur, but
>>> > if
>>> > you double-click on it, it's expanded to show all the context, location
>>> > and
>>> > all. You can't do that if the server doesn't send the context
>>> > information in
>>> > the first place.
>>> >
>>> >> I would be happy to show you the psql redirected output logs from my
>>> >> nightly server processes that spew into the megabytes because of
>>> >> logging various high level steps (did this, did that).
>>> >
>>> > Oh, I believe you. I understand what the problem is, we're only talking
>>> > about how best to address it.
>>>
>>> Yeah.  For posterity, a psql based solution would work fine for me,
>>> but a server side solution has a lot of advantages (less protocol
>>> chatter, more configurability, keeping libpq/psql light).
>>
>>
>> After some work on reduced version of "plpgsql.min_context" patch I am
>> inclining to think so ideal solution needs more steps - because this issue
>> has more than one dimension.
>>
>> There are two independent issues:
>>
>> 1. old plpgsql workaround that reduced the unwanted call stack info for
>> RAISE NOTICE. Negative side effect of this workaround is missing context
>> info about the RAISE command that raises the exception. We know a function,
>> but we don't know a line of related RAISE statement. The important is fact,
>> so NOTICE doesn't bubble to up. So this workaround was relative successful
>> without to implement some filtering on client or log side.
>
>
> I 

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-07-08 8:35 GMT+02:00 Pavel Stehule :

>
>
> 2015-07-07 18:15 GMT+02:00 Merlin Moncure :
>
>> On Tue, Jul 7, 2015 at 9:04 AM, Pavel Stehule 
>> wrote:
>> >> It doesn't have to if the behavior is guarded with a GUC.  I just
>> >> don't understand what all the fuss is about.  The default behavior of
>> >> logging that is well established by other languages (for example java)
>> >> that manage error stack for you should be to:
>> >>
>> >> *) Give stack trace when an uncaught exception is thrown
>> >> *) Do not give stack trace in all other logging cases unless asked for
>> >
>> > what is RAISE EXCEPTION - first or second case?
>>
>> First: RAISE (unless caught) is no different than any other kind of error.
>>
>> On Tue, Jul 7, 2015 at 9:42 AM, Heikki Linnakangas 
>> wrote:
>> > On 07/07/2015 04:56 PM, Merlin Moncure wrote:
>> >> It doesn't have to if the behavior is guarded with a GUC.  I just
>> >> don't understand what all the fuss is about.  The default behavior of
>> >> logging that is well established by other languages (for example java)
>> >> that manage error stack for you should be to:
>> >>
>> >> *) Give stack trace when an uncaught exception is thrown
>> >> *) Do not give stack trace in all other logging cases unless asked for
>> >
>> > Java's exception handling is so different from PostgreSQL's errors that
>> I
>> > don't think there's much to be learned from that. But I'll bite:
>> >
>> > First of all, Java's exceptions always contain a stack trace. It's up
>> to you
>> > when you catch an exception to decide whether to print it or not. "try
>> { ...
>> > } catch (Exception e) { e.printStackTrace() }" is fairly common,
>> actually.
>> > There is nothing like a NOTICE in Java, i.e. an exception that's thrown
>> but
>> > doesn't affect the control flow. The best I can think of is
>> > System.out.println(), which of course has no stack trace attached to it.
>>
>> exactly.
>>
>> > Perhaps you're arguing that NOTICE is more like printing to stderr, and
>> > should never contain any context information. I don't think that would
>> be an
>> > improvement. It's very handy to have the context information available
>> if
>> > don't know where a NOTICE is coming from, even if in most cases you're
>> not
>> > interested in it.
>>
>> That's exactly what I'm arguing.  NOTICE (and WARNING) are for
>> printing out information to client side logging; it's really the only
>> tool we have for that purpose and it fits that role perfectly.  Of
>> course, you may want to have NOTICE print context, especially when
>> debugging, but some control over that would be nice and in most cases
>> it's really not necessary.  I really don't understand the objection to
>> offering control over that behavior although I certainly understand
>> wanting to keep the default behavior as it currently is.
>>
>> > This is really quite different from a programming language's exception
>> > handling. First, there's a server, which produces the errors, and a
>> separate
>> > client, which displays them. You cannot catch an exception in the
>> client.
>> >
>> > BTW, let me throw in one use case to consider. We've been talking about
>> > psql, and what to print, but imagine a more sophisticated client like
>> > pgAdmin. It's not limited to either printing the context or not. It
>> could
>> > e.g. hide the context information of all messages when they occur, but
>> if
>> > you double-click on it, it's expanded to show all the context, location
>> and
>> > all. You can't do that if the server doesn't send the context
>> information in
>> > the first place.
>> >
>> >> I would be happy to show you the psql redirected output logs from my
>> >> nightly server processes that spew into the megabytes because of
>> >> logging various high level steps (did this, did that).
>> >
>> > Oh, I believe you. I understand what the problem is, we're only talking
>> > about how best to address it.
>>
>> Yeah.  For posterity, a psql based solution would work fine for me,
>> but a server side solution has a lot of advantages (less protocol
>> chatter, more configurability, keeping libpq/psql light).
>>
>
> After some work on reduced version of "plpgsql.min_context" patch I am
> inclining to think so ideal solution needs more steps - because this issue
> has more than one dimension.
>
> There are two independent issues:
>
> 1. old plpgsql workaround that reduced the unwanted call stack info for
> RAISE NOTICE. Negative side effect of this workaround is missing context
> info about the RAISE command that raises the exception. We know a function,
> but we don't know a line of related RAISE statement. The important is fact,
> so NOTICE doesn't bubble to up. So this workaround was relative successful
> without to implement some filtering on client or log side.
>

I found a other issue of this workaround - it doesn't work well for nested
SQL statement call, when inner statement invoke RAISE NOTICE. In this case
a context is showed too.

postgres=# insert into xx value

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Pavel Stehule]

Hi

here is initial version of reduced patch. It is small code, but relative
big (although I expected bigger) change in tests.

if these changes are too big, then we have to introduce a plpgsql GUC
plpgsql.client_min_context and plpgsql.log_min_client. These GUC overwrite
global setting for plpgsql functions. I'll be more happy without these
variables. It decrease a impact of changes, but there is not clean what
behave is expected when PL are used together - and when fails PLpgSQL
function called from PLPerl. The context filtering should be really solved
on TOP level.


Regards

Pavel

2015-07-08 14:09 GMT+02:00 Merlin Moncure :

> On Wed, Jul 8, 2015 at 1:35 AM, Pavel Stehule 
> wrote:
> >
> >
> > 2015-07-07 18:15 GMT+02:00 Merlin Moncure :
> >>
> >> On Tue, Jul 7, 2015 at 9:04 AM, Pavel Stehule 
> >> wrote:
> >> >> It doesn't have to if the behavior is guarded with a GUC.  I just
> >> >> don't understand what all the fuss is about.  The default behavior of
> >> >> logging that is well established by other languages (for example
> java)
> >> >> that manage error stack for you should be to:
> >> >>
> >> >> *) Give stack trace when an uncaught exception is thrown
> >> >> *) Do not give stack trace in all other logging cases unless asked
> for
> >> >
> >> > what is RAISE EXCEPTION - first or second case?
> >>
> >> First: RAISE (unless caught) is no different than any other kind of
> error.
> >>
> >> On Tue, Jul 7, 2015 at 9:42 AM, Heikki Linnakangas 
> >> wrote:
> >> > On 07/07/2015 04:56 PM, Merlin Moncure wrote:
> >> >> It doesn't have to if the behavior is guarded with a GUC.  I just
> >> >> don't understand what all the fuss is about.  The default behavior of
> >> >> logging that is well established by other languages (for example
> java)
> >> >> that manage error stack for you should be to:
> >> >>
> >> >> *) Give stack trace when an uncaught exception is thrown
> >> >> *) Do not give stack trace in all other logging cases unless asked
> for
> >> >
> >> > Java's exception handling is so different from PostgreSQL's errors
> that
> >> > I
> >> > don't think there's much to be learned from that. But I'll bite:
> >> >
> >> > First of all, Java's exceptions always contain a stack trace. It's up
> to
> >> > you
> >> > when you catch an exception to decide whether to print it or not.
> "try {
> >> > ...
> >> > } catch (Exception e) { e.printStackTrace() }" is fairly common,
> >> > actually.
> >> > There is nothing like a NOTICE in Java, i.e. an exception that's
> thrown
> >> > but
> >> > doesn't affect the control flow. The best I can think of is
> >> > System.out.println(), which of course has no stack trace attached to
> it.
> >>
> >> exactly.
> >>
> >> > Perhaps you're arguing that NOTICE is more like printing to stderr,
> and
> >> > should never contain any context information. I don't think that would
> >> > be an
> >> > improvement. It's very handy to have the context information available
> >> > if
> >> > don't know where a NOTICE is coming from, even if in most cases you're
> >> > not
> >> > interested in it.
> >>
> >> That's exactly what I'm arguing.  NOTICE (and WARNING) are for
> >> printing out information to client side logging; it's really the only
> >> tool we have for that purpose and it fits that role perfectly.  Of
> >> course, you may want to have NOTICE print context, especially when
> >> debugging, but some control over that would be nice and in most cases
> >> it's really not necessary.  I really don't understand the objection to
> >> offering control over that behavior although I certainly understand
> >> wanting to keep the default behavior as it currently is.
> >>
> >> > This is really quite different from a programming language's exception
> >> > handling. First, there's a server, which produces the errors, and a
> >> > separate
> >> > client, which displays them. You cannot catch an exception in the
> >> > client.
> >> >
> >> > BTW, let me throw in one use case to consider. We've been talking
> about
> >> > psql, and what to print, but imagine a more sophisticated client like
> >> > pgAdmin. It's not limited to either printing the context or not. It
> >> > could
> >> > e.g. hide the context information of all messages when they occur, but
> >> > if
> >> > you double-click on it, it's expanded to show all the context,
> location
> >> > and
> >> > all. You can't do that if the server doesn't send the context
> >> > information in
> >> > the first place.
> >> >
> >> >> I would be happy to show you the psql redirected output logs from my
> >> >> nightly server processes that spew into the megabytes because of
> >> >> logging various high level steps (did this, did that).
> >> >
> >> > Oh, I believe you. I understand what the problem is, we're only
> talking
> >> > about how best to address it.
> >>
> >> Yeah.  For posterity, a psql based solution would work fine for me,
> >> but a server side solution has a lot of advantages (less protocol
> >> chatter, more configurability, keeping libp

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Merlin Moncure]

On Wed, Jul 8, 2015 at 1:35 AM, Pavel Stehule  wrote:
>
>
> 2015-07-07 18:15 GMT+02:00 Merlin Moncure :
>>
>> On Tue, Jul 7, 2015 at 9:04 AM, Pavel Stehule 
>> wrote:
>> >> It doesn't have to if the behavior is guarded with a GUC.  I just
>> >> don't understand what all the fuss is about.  The default behavior of
>> >> logging that is well established by other languages (for example java)
>> >> that manage error stack for you should be to:
>> >>
>> >> *) Give stack trace when an uncaught exception is thrown
>> >> *) Do not give stack trace in all other logging cases unless asked for
>> >
>> > what is RAISE EXCEPTION - first or second case?
>>
>> First: RAISE (unless caught) is no different than any other kind of error.
>>
>> On Tue, Jul 7, 2015 at 9:42 AM, Heikki Linnakangas 
>> wrote:
>> > On 07/07/2015 04:56 PM, Merlin Moncure wrote:
>> >> It doesn't have to if the behavior is guarded with a GUC.  I just
>> >> don't understand what all the fuss is about.  The default behavior of
>> >> logging that is well established by other languages (for example java)
>> >> that manage error stack for you should be to:
>> >>
>> >> *) Give stack trace when an uncaught exception is thrown
>> >> *) Do not give stack trace in all other logging cases unless asked for
>> >
>> > Java's exception handling is so different from PostgreSQL's errors that
>> > I
>> > don't think there's much to be learned from that. But I'll bite:
>> >
>> > First of all, Java's exceptions always contain a stack trace. It's up to
>> > you
>> > when you catch an exception to decide whether to print it or not. "try {
>> > ...
>> > } catch (Exception e) { e.printStackTrace() }" is fairly common,
>> > actually.
>> > There is nothing like a NOTICE in Java, i.e. an exception that's thrown
>> > but
>> > doesn't affect the control flow. The best I can think of is
>> > System.out.println(), which of course has no stack trace attached to it.
>>
>> exactly.
>>
>> > Perhaps you're arguing that NOTICE is more like printing to stderr, and
>> > should never contain any context information. I don't think that would
>> > be an
>> > improvement. It's very handy to have the context information available
>> > if
>> > don't know where a NOTICE is coming from, even if in most cases you're
>> > not
>> > interested in it.
>>
>> That's exactly what I'm arguing.  NOTICE (and WARNING) are for
>> printing out information to client side logging; it's really the only
>> tool we have for that purpose and it fits that role perfectly.  Of
>> course, you may want to have NOTICE print context, especially when
>> debugging, but some control over that would be nice and in most cases
>> it's really not necessary.  I really don't understand the objection to
>> offering control over that behavior although I certainly understand
>> wanting to keep the default behavior as it currently is.
>>
>> > This is really quite different from a programming language's exception
>> > handling. First, there's a server, which produces the errors, and a
>> > separate
>> > client, which displays them. You cannot catch an exception in the
>> > client.
>> >
>> > BTW, let me throw in one use case to consider. We've been talking about
>> > psql, and what to print, but imagine a more sophisticated client like
>> > pgAdmin. It's not limited to either printing the context or not. It
>> > could
>> > e.g. hide the context information of all messages when they occur, but
>> > if
>> > you double-click on it, it's expanded to show all the context, location
>> > and
>> > all. You can't do that if the server doesn't send the context
>> > information in
>> > the first place.
>> >
>> >> I would be happy to show you the psql redirected output logs from my
>> >> nightly server processes that spew into the megabytes because of
>> >> logging various high level steps (did this, did that).
>> >
>> > Oh, I believe you. I understand what the problem is, we're only talking
>> > about how best to address it.
>>
>> Yeah.  For posterity, a psql based solution would work fine for me,
>> but a server side solution has a lot of advantages (less protocol
>> chatter, more configurability, keeping libpq/psql light).
>
>
> After some work on reduced version of "plpgsql.min_context" patch I am
> inclining to think so ideal solution needs more steps - because this issue
> has more than one dimension.
>
> There are two independent issues:
>
> 1. old plpgsql workaround that reduced the unwanted call stack info for
> RAISE NOTICE. Negative side effect of this workaround is missing context
> info about the RAISE command that raises the exception. We know a function,
> but we don't know a line of related RAISE statement. The important is fact,
> so NOTICE doesn't bubble to up. So this workaround was relative successful
> without to implement some filtering on client or log side.
>
> 2. second issue is general suppressing context info for interactive client
> or for log.
>
> These issues should be solved separately, because solution for @2 doesn't

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-07-07 18:15 GMT+02:00 Merlin Moncure :

> On Tue, Jul 7, 2015 at 9:04 AM, Pavel Stehule 
> wrote:
> >> It doesn't have to if the behavior is guarded with a GUC.  I just
> >> don't understand what all the fuss is about.  The default behavior of
> >> logging that is well established by other languages (for example java)
> >> that manage error stack for you should be to:
> >>
> >> *) Give stack trace when an uncaught exception is thrown
> >> *) Do not give stack trace in all other logging cases unless asked for
> >
> > what is RAISE EXCEPTION - first or second case?
>
> First: RAISE (unless caught) is no different than any other kind of error.
>
> On Tue, Jul 7, 2015 at 9:42 AM, Heikki Linnakangas 
> wrote:
> > On 07/07/2015 04:56 PM, Merlin Moncure wrote:
> >> It doesn't have to if the behavior is guarded with a GUC.  I just
> >> don't understand what all the fuss is about.  The default behavior of
> >> logging that is well established by other languages (for example java)
> >> that manage error stack for you should be to:
> >>
> >> *) Give stack trace when an uncaught exception is thrown
> >> *) Do not give stack trace in all other logging cases unless asked for
> >
> > Java's exception handling is so different from PostgreSQL's errors that I
> > don't think there's much to be learned from that. But I'll bite:
> >
> > First of all, Java's exceptions always contain a stack trace. It's up to
> you
> > when you catch an exception to decide whether to print it or not. "try {
> ...
> > } catch (Exception e) { e.printStackTrace() }" is fairly common,
> actually.
> > There is nothing like a NOTICE in Java, i.e. an exception that's thrown
> but
> > doesn't affect the control flow. The best I can think of is
> > System.out.println(), which of course has no stack trace attached to it.
>
> exactly.
>
> > Perhaps you're arguing that NOTICE is more like printing to stderr, and
> > should never contain any context information. I don't think that would
> be an
> > improvement. It's very handy to have the context information available if
> > don't know where a NOTICE is coming from, even if in most cases you're
> not
> > interested in it.
>
> That's exactly what I'm arguing.  NOTICE (and WARNING) are for
> printing out information to client side logging; it's really the only
> tool we have for that purpose and it fits that role perfectly.  Of
> course, you may want to have NOTICE print context, especially when
> debugging, but some control over that would be nice and in most cases
> it's really not necessary.  I really don't understand the objection to
> offering control over that behavior although I certainly understand
> wanting to keep the default behavior as it currently is.
>
> > This is really quite different from a programming language's exception
> > handling. First, there's a server, which produces the errors, and a
> separate
> > client, which displays them. You cannot catch an exception in the client.
> >
> > BTW, let me throw in one use case to consider. We've been talking about
> > psql, and what to print, but imagine a more sophisticated client like
> > pgAdmin. It's not limited to either printing the context or not. It could
> > e.g. hide the context information of all messages when they occur, but if
> > you double-click on it, it's expanded to show all the context, location
> and
> > all. You can't do that if the server doesn't send the context
> information in
> > the first place.
> >
> >> I would be happy to show you the psql redirected output logs from my
> >> nightly server processes that spew into the megabytes because of
> >> logging various high level steps (did this, did that).
> >
> > Oh, I believe you. I understand what the problem is, we're only talking
> > about how best to address it.
>
> Yeah.  For posterity, a psql based solution would work fine for me,
> but a server side solution has a lot of advantages (less protocol
> chatter, more configurability, keeping libpq/psql light).
>

After some work on reduced version of "plpgsql.min_context" patch I am
inclining to think so ideal solution needs more steps - because this issue
has more than one dimension.

There are two independent issues:

1. old plpgsql workaround that reduced the unwanted call stack info for
RAISE NOTICE. Negative side effect of this workaround is missing context
info about the RAISE command that raises the exception. We know a function,
but we don't know a line of related RAISE statement. The important is fact,
so NOTICE doesn't bubble to up. So this workaround was relative successful
without to implement some filtering on client or log side.

2. second issue is general suppressing context info for interactive client
or for log.

These issues should be solved separately, because solution for @2 doesn't
fix @1, and @1 is too local for @2.

So what we can do?

1. remove current plpgsql workaround - and implement client_min_context and
log_min_context
2. implement plpgsql.min_context, and client_min_context and log_min_c

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-07-07 18:15 GMT+02:00 Merlin Moncure :

> On Tue, Jul 7, 2015 at 9:04 AM, Pavel Stehule 
> wrote:
> >> It doesn't have to if the behavior is guarded with a GUC.  I just
> >> don't understand what all the fuss is about.  The default behavior of
> >> logging that is well established by other languages (for example java)
> >> that manage error stack for you should be to:
> >>
> >> *) Give stack trace when an uncaught exception is thrown
> >> *) Do not give stack trace in all other logging cases unless asked for
> >
> > what is RAISE EXCEPTION - first or second case?
>
> First: RAISE (unless caught) is no different than any other kind of error.
>
> On Tue, Jul 7, 2015 at 9:42 AM, Heikki Linnakangas 
> wrote:
> > On 07/07/2015 04:56 PM, Merlin Moncure wrote:
> >> It doesn't have to if the behavior is guarded with a GUC.  I just
> >> don't understand what all the fuss is about.  The default behavior of
> >> logging that is well established by other languages (for example java)
> >> that manage error stack for you should be to:
> >>
> >> *) Give stack trace when an uncaught exception is thrown
> >> *) Do not give stack trace in all other logging cases unless asked for
> >
> > Java's exception handling is so different from PostgreSQL's errors that I
> > don't think there's much to be learned from that. But I'll bite:
> >
> > First of all, Java's exceptions always contain a stack trace. It's up to
> you
> > when you catch an exception to decide whether to print it or not. "try {
> ...
> > } catch (Exception e) { e.printStackTrace() }" is fairly common,
> actually.
> > There is nothing like a NOTICE in Java, i.e. an exception that's thrown
> but
> > doesn't affect the control flow. The best I can think of is
> > System.out.println(), which of course has no stack trace attached to it.
>
> exactly.
>
> > Perhaps you're arguing that NOTICE is more like printing to stderr, and
> > should never contain any context information. I don't think that would
> be an
> > improvement. It's very handy to have the context information available if
> > don't know where a NOTICE is coming from, even if in most cases you're
> not
> > interested in it.
>
> That's exactly what I'm arguing.  NOTICE (and WARNING) are for
> printing out information to client side logging; it's really the only
> tool we have for that purpose and it fits that role perfectly.  Of
> course, you may want to have NOTICE print context, especially when
> debugging, but some control over that would be nice and in most cases
> it's really not necessary.  I really don't understand the objection to
> offering control over that behavior although I certainly understand
> wanting to keep the default behavior as it currently is.
>
> > This is really quite different from a programming language's exception
> > handling. First, there's a server, which produces the errors, and a
> separate
> > client, which displays them. You cannot catch an exception in the client.
> >
> > BTW, let me throw in one use case to consider. We've been talking about
> > psql, and what to print, but imagine a more sophisticated client like
> > pgAdmin. It's not limited to either printing the context or not. It could
> > e.g. hide the context information of all messages when they occur, but if
> > you double-click on it, it's expanded to show all the context, location
> and
> > all. You can't do that if the server doesn't send the context
> information in
> > the first place.
> >
> >> I would be happy to show you the psql redirected output logs from my
> >> nightly server processes that spew into the megabytes because of
> >> logging various high level steps (did this, did that).
> >
> > Oh, I believe you. I understand what the problem is, we're only talking
> > about how best to address it.
>
> Yeah.  For posterity, a psql based solution would work fine for me,
> but a server side solution has a lot of advantages (less protocol
> chatter, more configurability, keeping libpq/psql light).
>

I prefer a server side solution too. With it I can have (as plpgsql
developer) bigger control of expected output.

Client can change this behave on global (min_context) or on language level
(plpgsql.min_context). If somebody afraid about security, we can to enforce
rule so min_context <= error always.

The possibility to enable or disable context per any RAISE statement is
nice to have, but it is not fundamental.

Other variant is a implementation of min_context on client side -  but then
we cannot to ensure current behave and fix plpgsql raise exception issue
together.

Pavel


>
> merlin
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Merlin Moncure]

On Tue, Jul 7, 2015 at 9:04 AM, Pavel Stehule  wrote:
>> It doesn't have to if the behavior is guarded with a GUC.  I just
>> don't understand what all the fuss is about.  The default behavior of
>> logging that is well established by other languages (for example java)
>> that manage error stack for you should be to:
>>
>> *) Give stack trace when an uncaught exception is thrown
>> *) Do not give stack trace in all other logging cases unless asked for
>
> what is RAISE EXCEPTION - first or second case?

First: RAISE (unless caught) is no different than any other kind of error.

On Tue, Jul 7, 2015 at 9:42 AM, Heikki Linnakangas  wrote:
> On 07/07/2015 04:56 PM, Merlin Moncure wrote:
>> It doesn't have to if the behavior is guarded with a GUC.  I just
>> don't understand what all the fuss is about.  The default behavior of
>> logging that is well established by other languages (for example java)
>> that manage error stack for you should be to:
>>
>> *) Give stack trace when an uncaught exception is thrown
>> *) Do not give stack trace in all other logging cases unless asked for
>
> Java's exception handling is so different from PostgreSQL's errors that I
> don't think there's much to be learned from that. But I'll bite:
>
> First of all, Java's exceptions always contain a stack trace. It's up to you
> when you catch an exception to decide whether to print it or not. "try { ...
> } catch (Exception e) { e.printStackTrace() }" is fairly common, actually.
> There is nothing like a NOTICE in Java, i.e. an exception that's thrown but
> doesn't affect the control flow. The best I can think of is
> System.out.println(), which of course has no stack trace attached to it.

exactly.

> Perhaps you're arguing that NOTICE is more like printing to stderr, and
> should never contain any context information. I don't think that would be an
> improvement. It's very handy to have the context information available if
> don't know where a NOTICE is coming from, even if in most cases you're not
> interested in it.

That's exactly what I'm arguing.  NOTICE (and WARNING) are for
printing out information to client side logging; it's really the only
tool we have for that purpose and it fits that role perfectly.  Of
course, you may want to have NOTICE print context, especially when
debugging, but some control over that would be nice and in most cases
it's really not necessary.  I really don't understand the objection to
offering control over that behavior although I certainly understand
wanting to keep the default behavior as it currently is.

> This is really quite different from a programming language's exception
> handling. First, there's a server, which produces the errors, and a separate
> client, which displays them. You cannot catch an exception in the client.
>
> BTW, let me throw in one use case to consider. We've been talking about
> psql, and what to print, but imagine a more sophisticated client like
> pgAdmin. It's not limited to either printing the context or not. It could
> e.g. hide the context information of all messages when they occur, but if
> you double-click on it, it's expanded to show all the context, location and
> all. You can't do that if the server doesn't send the context information in
> the first place.
>
>> I would be happy to show you the psql redirected output logs from my
>> nightly server processes that spew into the megabytes because of
>> logging various high level steps (did this, did that).
>
> Oh, I believe you. I understand what the problem is, we're only talking
> about how best to address it.

Yeah.  For posterity, a psql based solution would work fine for me,
but a server side solution has a lot of advantages (less protocol
chatter, more configurability, keeping libpq/psql light).

merlin


-- 
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] PL/pgSQL, RAISE and error context

[Heikki Linnakangas]

On 07/07/2015 04:56 PM, Merlin Moncure wrote:

On Tue, Jul 7, 2015 at 8:13 AM, Heikki Linnakangas  wrote:

On 01/26/2015 05:14 PM, Tom Lane wrote:


Pavel Stehule  writes:


2015-01-26 14:02 GMT+01:00 Marko Tiikkaja :
I am thinking, so solution




   /* if we are doing RAISE, don't report its location */
  if (estate->err_text == raise_skip_msg)
  return;




is too simple, and this part should be fixed. This change can be done by
on
plpgsql or libpq side. This is bug, and it should be fixed.



Doing this in libpq is utterly insane.  It has not got sufficient context
to do anything intelligent.  The fact that it's not intelligent is exposed
by the regression test changes that the proposed patch causes, most of
which do not look like improvements.


How can the server know if the client wants to display context information?


It doesn't have to if the behavior is guarded with a GUC.  I just
don't understand what all the fuss is about.  The default behavior of
logging that is well established by other languages (for example java)
that manage error stack for you should be to:

*) Give stack trace when an uncaught exception is thrown
*) Do not give stack trace in all other logging cases unless asked for


Java's exception handling is so different from PostgreSQL's errors that 
I don't think there's much to be learned from that. But I'll bite:


First of all, Java's exceptions always contain a stack trace. It's up to 
you when you catch an exception to decide whether to print it or not. 
"try { ... } catch (Exception e) { e.printStackTrace() }" is fairly 
common, actually. There is nothing like a NOTICE in Java, i.e. an 
exception that's thrown but doesn't affect the control flow. The best I 
can think of is System.out.println(), which of course has no stack trace 
attached to it.


Perhaps you're arguing that NOTICE is more like printing to stderr, and 
should never contain any context information. I don't think that would 
be an improvement. It's very handy to have the context information 
available if don't know where a NOTICE is coming from, even if in most 
cases you're not interested in it.


This is really quite different from a programming language's exception 
handling. First, there's a server, which produces the errors, and a 
separate client, which displays them. You cannot catch an exception in 
the client.


BTW, let me throw in one use case to consider. We've been talking about 
psql, and what to print, but imagine a more sophisticated client like 
pgAdmin. It's not limited to either printing the context or not. It 
could e.g. hide the context information of all messages when they occur, 
but if you double-click on it, it's expanded to show all the context, 
location and all. You can't do that if the server doesn't send the 
context information in the first place.



I would be happy to show you the psql redirected output logs from my
nightly server processes that spew into the megabytes because of
logging various high level steps (did this, did that).


Oh, I believe you. I understand what the problem is, we're only talking 
about how best to address it.

- Heikki



--
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-07-07 15:56 GMT+02:00 Merlin Moncure :

> On Tue, Jul 7, 2015 at 8:13 AM, Heikki Linnakangas 
> wrote:
> > On 01/26/2015 05:14 PM, Tom Lane wrote:
> >>
> >> Pavel Stehule  writes:
> >>>
> >>> 2015-01-26 14:02 GMT+01:00 Marko Tiikkaja :
> >>> I am thinking, so solution
> >>
> >>
> >>>   /* if we are doing RAISE, don't report its location */
> >>>  if (estate->err_text == raise_skip_msg)
> >>>  return;
> >>
> >>
> >>> is too simple, and this part should be fixed. This change can be done
> by
> >>> on
> >>> plpgsql or libpq side. This is bug, and it should be fixed.
> >>
> >>
> >> Doing this in libpq is utterly insane.  It has not got sufficient
> context
> >> to do anything intelligent.  The fact that it's not intelligent is
> exposed
> >> by the regression test changes that the proposed patch causes, most of
> >> which do not look like improvements.
> >
> > How can the server know if the client wants to display context
> information?
>
> It doesn't have to if the behavior is guarded with a GUC.  I just
> don't understand what all the fuss is about.  The default behavior of
> logging that is well established by other languages (for example java)
> that manage error stack for you should be to:
>
> *) Give stack trace when an uncaught exception is thrown
> *) Do not give stack trace in all other logging cases unless asked for
>

what is RAISE EXCEPTION - first or second case?


>
> I would be happy to show you the psql redirected output logs from my
> nightly server processes that spew into the megabytes because of
> logging various high level steps (did this, did that).   I can't throw
> the verbose switch to terse because if the error happens to be
> 'Division by Zero', or some other difficult to trace problem then I'm
> sunk.  I believe the protocol decision to 'always send context' needs
> to be revisited; if your server-side codebase is large and heavily
> nested it makes logging an expensive operation even if the client
> strips off the log.
>
> plpgsql.min_context seems like the ideal solution to this problem; it
> can be managed on the server or the client and does not require new
> syntax.  If we require syntax to slip and and out of debugging type
> operations the solution has missed the mark IMNSHO.
>
> merlin
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Merlin Moncure]

On Tue, Jul 7, 2015 at 8:13 AM, Heikki Linnakangas  wrote:
> On 01/26/2015 05:14 PM, Tom Lane wrote:
>>
>> Pavel Stehule  writes:
>>>
>>> 2015-01-26 14:02 GMT+01:00 Marko Tiikkaja :
>>> I am thinking, so solution
>>
>>
>>>   /* if we are doing RAISE, don't report its location */
>>>  if (estate->err_text == raise_skip_msg)
>>>  return;
>>
>>
>>> is too simple, and this part should be fixed. This change can be done by
>>> on
>>> plpgsql or libpq side. This is bug, and it should be fixed.
>>
>>
>> Doing this in libpq is utterly insane.  It has not got sufficient context
>> to do anything intelligent.  The fact that it's not intelligent is exposed
>> by the regression test changes that the proposed patch causes, most of
>> which do not look like improvements.
>
> How can the server know if the client wants to display context information?

It doesn't have to if the behavior is guarded with a GUC.  I just
don't understand what all the fuss is about.  The default behavior of
logging that is well established by other languages (for example java)
that manage error stack for you should be to:

*) Give stack trace when an uncaught exception is thrown
*) Do not give stack trace in all other logging cases unless asked for

I would be happy to show you the psql redirected output logs from my
nightly server processes that spew into the megabytes because of
logging various high level steps (did this, did that).   I can't throw
the verbose switch to terse because if the error happens to be
'Division by Zero', or some other difficult to trace problem then I'm
sunk.  I believe the protocol decision to 'always send context' needs
to be revisited; if your server-side codebase is large and heavily
nested it makes logging an expensive operation even if the client
strips off the log.

plpgsql.min_context seems like the ideal solution to this problem; it
can be managed on the server or the client and does not require new
syntax.  If we require syntax to slip and and out of debugging type
operations the solution has missed the mark IMNSHO.

merlin


-- 
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] PL/pgSQL, RAISE and error context

[Heikki Linnakangas]

On 01/26/2015 05:14 PM, Tom Lane wrote:

Pavel Stehule  writes:

2015-01-26 14:02 GMT+01:00 Marko Tiikkaja :
I am thinking, so solution



  /* if we are doing RAISE, don't report its location */
 if (estate->err_text == raise_skip_msg)
 return;



is too simple, and this part should be fixed. This change can be done by on
plpgsql or libpq side. This is bug, and it should be fixed.


Doing this in libpq is utterly insane.  It has not got sufficient context
to do anything intelligent.  The fact that it's not intelligent is exposed
by the regression test changes that the proposed patch causes, most of
which do not look like improvements.


I think doing this in libpq (or psql) is the way to go. How can the 
server know if the client wants to display context information? We just 
have to make sure the client has enough information to make a smart 
decision. If the client doesn't have enough information today, then 
let's work on that.


Note that Marko's patch didn't change libpq's default printing mode, 
which is why you got all the extra CONTEXT lines in the regression tests 
that were not there before. Just as if we just removed the suppression 
from PL/pgSQL and did nothing else. I think we need to also change the 
default behaviour to not print CONTEXT lines for NOTICE-level messages, 
getting us closer to the current behaviour again.


If you run the regression tests in the "compact" verbosity, the 
regression test output changes look quite sensible to me. See attached.



Another problem is that past requests to change this behavior have
generally been to the effect that people wanted *more* context suppressed
not less, ie they didn't want any CONTEXT lines at all on certain
messages.  So the proposed patch seems to me to be going in exactly the
wrong direction.


After changing the default to "compact", it prints less CONTEXT lines.


The design I thought had been agreed on was to add some new option to
plpgsql's RAISE command which would cause suppression of all CONTEXT lines
not just the most closely nested one.


I don't understand how you came to that conclusion. In particular, see 
http://www.postgresql.org/message-id/6656.1377100...@sss.pgh.pa.us. If 
you changed your mind, you forgot to tell why.


- Heikki
*** 
/home/heikki/git-sandbox-pgsql/master/src/test/regress/expected/triggers.out
2015-07-07 15:40:38.697861317 +0300
--- /home/heikki/git-sandbox-pgsql/master/src/test/regress/results/triggers.out 
2015-07-07 15:50:34.885805473 +0300
***
*** 958,968 
  NOTICE:  main_view INSTEAD OF INSERT ROW (instead_of_ins)
  NOTICE:  NEW: (20,30)
  NOTICE:  trigger_func(before_ins_stmt) called: action = INSERT, when = 
BEFORE, level = STATEMENT
- CONTEXT:  SQL statement "INSERT INTO main_table VALUES (NEW.a, NEW.b)"
- PL/pgSQL function view_trigger() line 17 at SQL statement
  NOTICE:  trigger_func(after_ins_stmt) called: action = INSERT, when = AFTER, 
level = STATEMENT
- CONTEXT:  SQL statement "INSERT INTO main_table VALUES (NEW.a, NEW.b)"
- PL/pgSQL function view_trigger() line 17 at SQL statement
  NOTICE:  main_view AFTER INSERT STATEMENT (after_view_ins_stmt)
  INSERT 0 1
  INSERT INTO main_view VALUES (21, 31) RETURNING a, b;
--- 958,964 
***
*** 970,980 
  NOTICE:  main_view INSTEAD OF INSERT ROW (instead_of_ins)
  NOTICE:  NEW: (21,31)
  NOTICE:  trigger_func(before_ins_stmt) called: action = INSERT, when = 
BEFORE, level = STATEMENT
- CONTEXT:  SQL statement "INSERT INTO main_table VALUES (NEW.a, NEW.b)"
- PL/pgSQL function view_trigger() line 17 at SQL statement
  NOTICE:  trigger_func(after_ins_stmt) called: action = INSERT, when = AFTER, 
level = STATEMENT
- CONTEXT:  SQL statement "INSERT INTO main_table VALUES (NEW.a, NEW.b)"
- PL/pgSQL function view_trigger() line 17 at SQL statement
  NOTICE:  main_view AFTER INSERT STATEMENT (after_view_ins_stmt)
   a  | b  
  +
--- 966,972 
***
*** 988,1004 
  NOTICE:  main_view INSTEAD OF UPDATE ROW (instead_of_upd)
  NOTICE:  OLD: (20,30), NEW: (20,31)
  NOTICE:  trigger_func(before_upd_a_stmt) called: action = UPDATE, when = 
BEFORE, level = STATEMENT
- CONTEXT:  SQL statement "UPDATE main_table SET a = NEW.a, b = NEW.b WHERE a = 
OLD.a AND b = OLD.b"
- PL/pgSQL function view_trigger() line 23 at SQL statement
  NOTICE:  trigger_func(before_upd_a_row) called: action = UPDATE, when = 
BEFORE, level = ROW
- CONTEXT:  SQL statement "UPDATE main_table SET a = NEW.a, b = NEW.b WHERE a = 
OLD.a AND b = OLD.b"
- PL/pgSQL function view_trigger() line 23 at SQL statement
  NOTICE:  trigger_func(after_upd_b_stmt) called: action = UPDATE, when = 
AFTER, level = STATEMENT
- CONTEXT:  SQL statement "UPDATE main_table SET a = NEW.a, b = NEW.b WHERE a = 
OLD.a AND b = OLD.b"
- PL/pgSQL function view_trigger() line 23 at SQL statement
  NOTICE:  trigger_func(after_upd_stmt) called: action = UPDATE, when = AFTER, 
level = STATEMENT
- CONTEXT:  SQL statement "UPDATE main_table S

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-04-30 10:50 GMT+02:00 Pavel Stehule :

>
>
> 2015-04-30 10:24 GMT+02:00 Marko Tiikkaja :
>
>> Hi Pavel,
>>
>> This doesn't seem to be what I thought we had agreed on.  For example:
>>
>> =# create function barf() returns void as $$ begin raise notice without
>> context 'hello world'; end $$ language plpgsql;
>> CREATE FUNCTION
>> =# create function foof() returns void as $$ begin perform barf(); end $$
>> language plpgsql;
>> CREATE FUNCTION
>> =# select foof();
>> NOTICE:  hello world
>> CONTEXT:  SQL statement "SELECT barf()"
>> PL/pgSQL function foof() line 1 at PERFORM
>>
>> It's not only clear that WITHOUT CONTEXT didn't really work here, but it
>> also had absolutely no effect since the context within barf() is also
>> displayed.
>>
>
> It doesn't look well - because it should be solve by errhidecontext(true)
>
> yes, there is a issue in send_message_to_frontend - this ignore
> edata->hide_ctx field. After fixing, it working as expected - so this is a
> bug in implementation of errhidecontext()
>
> should be
>
> if (edata->context && !edata->hide_ctx)
> {
> pq_sendbyte(&msgbuf, PG_DIAG_CONTEXT);
> err_sendstring(&msgbuf, edata->context);
> }
>
> and probably getting stack in err_finish should be fixed too:
>
> if (!edata->hide_ctx)
> for (econtext = error_context_stack;
>  econtext != NULL;
>  econtext = econtext->previous)
> (*econtext->callback) (econtext->arg);
>
> Regards
>
> Pavel
>
>
I am sending patch


>
>
> I'll look on this issue.
>
> Regards
>
> Pavel
>
>
>>
>>
>> .m
>>
>
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-04-30 10:24 GMT+02:00 Marko Tiikkaja :

> Hi Pavel,
>
> This doesn't seem to be what I thought we had agreed on.  For example:
>
> =# create function barf() returns void as $$ begin raise notice without
> context 'hello world'; end $$ language plpgsql;
> CREATE FUNCTION
> =# create function foof() returns void as $$ begin perform barf(); end $$
> language plpgsql;
> CREATE FUNCTION
> =# select foof();
> NOTICE:  hello world
> CONTEXT:  SQL statement "SELECT barf()"
> PL/pgSQL function foof() line 1 at PERFORM
>
> It's not only clear that WITHOUT CONTEXT didn't really work here, but it
> also had absolutely no effect since the context within barf() is also
> displayed.
>

It doesn't look well - because it should be solve by errhidecontext(true)

yes, there is a issue in send_message_to_frontend - this ignore
edata->hide_ctx field. After fixing, it working as expected - so this is a
bug in implementation of errhidecontext()

should be

if (edata->context && !edata->hide_ctx)
{
pq_sendbyte(&msgbuf, PG_DIAG_CONTEXT);
err_sendstring(&msgbuf, edata->context);
}

and probably getting stack in err_finish should be fixed too:

if (!edata->hide_ctx)
for (econtext = error_context_stack;
 econtext != NULL;
 econtext = econtext->previous)
(*econtext->callback) (econtext->arg);

Regards

Pavel



I'll look on this issue.

Regards

Pavel


>
>
> .m
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Marko Tiikkaja]

Hi Pavel,

This doesn't seem to be what I thought we had agreed on.  For example:

=# create function barf() returns void as $$ begin raise notice without 
context 'hello world'; end $$ language plpgsql;

CREATE FUNCTION
=# create function foof() returns void as $$ begin perform barf(); end 
$$ language plpgsql;

CREATE FUNCTION
=# select foof();
NOTICE:  hello world
CONTEXT:  SQL statement "SELECT barf()"
PL/pgSQL function foof() line 1 at PERFORM

It's not only clear that WITHOUT CONTEXT didn't really work here, but it 
also had absolutely no effect since the context within barf() is also 
displayed.



.m


--
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-04-28 19:44 GMT+02:00 Jim Nasby :

> On 4/28/15 1:16 AM, Pavel Stehule wrote:
>
>>
>> I think it can't be any clearer than the proposed
>> "plpgsql.display_context_min_messages"
>>
>>
>> client_min_context. It's doing the same thing as min_messages does,
>> just for context instead of the message.
>>
>> Or does this affect client and log the same way?
>>
>>
>> it affect client and log together
>>
>> maybe "min_context"
>>
>
> +1


third variant with GUC plpgsql.min_context

Regards

Pavel


>
> --
> Jim Nasby, Data Architect, Blue Treble Consulting
> Data in Trouble? Get it in Treble! http://BlueTreble.com
>
commit c2f49938f636864234d03994d2f64f8095392d11
Author: Pavel Stehule 
Date:   Sat Apr 25 22:09:28 2015 +0200

initial implementation of (WITH|WITHOUT) CONTEXT clause to plpgsql RAISE statement.

initial implementation of plpgsql GUC plpgsql.min_context

diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index d36acf6..ffc3eb8 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -3406,10 +3406,10 @@ END LOOP  label ;
 raise errors.
 
 
-RAISE  level  'format' , expression , ...   USING option = expression , ...  ;
-RAISE  level  condition_name  USING option = expression , ...  ;
-RAISE  level  SQLSTATE 'sqlstate'  USING option = expression , ...  ;
-RAISE  level  USING option = expression , ... ;
+RAISE  level   ( WITH | WITHOUT ) CONTEXT  'format' , expression , ...   USING option = expression , ...  ;
+RAISE  level   ( WITH | WITHOUT ) CONTEXT  condition_name  USING option = expression , ...  ;
+RAISE  level   ( WITH | WITHOUT ) CONTEXT  SQLSTATE 'sqlstate'  USING option = expression , ...  ;
+RAISE  level   ( WITH | WITHOUT ) CONTEXT  USING option = expression , ... ;
 RAISE ;
 
 
@@ -3431,6 +3431,18 @@ RAISE ;

 

+The options WITH CONTEXT or WITHOUT CONTEXT
+can enforce or suppress context information related to error or notice. This possibility
+can be forced by settings of configuration parameter plpgsql.min_context.
+This allows same values like level option plus
+value none that is a default. When it is changed, then all errors and notices
+with higher than specified severity are raised with context info.
+
+RAISE NOTICE WITH CONTEXT 'This message will have a context';
+
+   
+
+   
 After level if any,
 you can write a format
 (which must be a simple string literal, not an expression).  The
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index deefb1f..eaee5a7 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2921,6 +2921,7 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
 	char	   *err_table = NULL;
 	char	   *err_schema = NULL;
 	ListCell   *lc;
+	bool			hide_ctx = true;		/* suppress context by default */
 
 	/* RAISE with no parameters: re-throw current exception */
 	if (stmt->condname == NULL && stmt->message == NULL &&
@@ -3080,10 +3081,16 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
 			err_message = pstrdup(unpack_sql_state(err_code));
 	}
 
-	/*
-	 * Throw the error (may or may not come back)
-	 */
-	estate->err_text = raise_skip_msg;	/* suppress traceback of raise */
+	if (stmt->context_info == PLPGSQL_CONTEXT_DISPLAY)
+		hide_ctx = false;
+	else if (stmt->context_info == PLPGSQL_CONTEXT_DEFAULT)
+	{
+		if (plpgsql_min_context != PLPGSQL_MIN_CONTEXT_SUPPRESS)
+			hide_ctx = stmt->elog_level < plpgsql_min_context;
+	}
+
+	if (hide_ctx)
+		estate->err_text = raise_skip_msg;
 
 	ereport(stmt->elog_level,
 			(err_code ? errcode(err_code) : 0,
@@ -3099,7 +3106,8 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
 			 (err_table != NULL) ?
 			 err_generic_string(PG_DIAG_TABLE_NAME, err_table) : 0,
 			 (err_schema != NULL) ?
-			 err_generic_string(PG_DIAG_SCHEMA_NAME, err_schema) : 0));
+			 err_generic_string(PG_DIAG_SCHEMA_NAME, err_schema) : 0,
+			 errhidecontext(hide_ctx)));
 
 	estate->err_text = NULL;	/* un-suppress... */
 
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 4026e41..48914a7 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -259,6 +259,7 @@ static	void			check_raise_parameters(PLpgSQL_stmt_raise *stmt);
 %token 	K_CONSTANT
 %token 	K_CONSTRAINT
 %token 	K_CONSTRAINT_NAME
+%token 	K_CONTEXT
 %token 	K_CONTINUE
 %token 	K_CURRENT
 %token 	K_CURSOR
@@ -341,6 +342,8 @@ static	void			check_raise_parameters(PLpgSQL_stmt_raise *stmt);
 %token 	K_WARNING
 %token 	K_WHEN
 %token 	K_WHILE
+%token 	K_WITH
+%token 	K_WITHOUT
 
 %%
 
@@ -1716,6 +1719,7 @@ stmt_raise		: K_RAISE
 		new->cmd_type	= PLPGSQL_STMT_RAISE;
 		new->lineno		= plpgsql_location_to_lineno(@1);
 		new->elog_level = ERROR;	/* default */
+		new->context_info = PLPGSQL_CONTEXT_DEFAULT;
 		new->condname	= NULL;
 		new->message	= NULL;
 		new->params		= NIL;
@@ -1773,6 +1777

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Jim Nasby]

On 4/28/15 1:16 AM, Pavel Stehule wrote:


I think it can't be any clearer than the proposed
"plpgsql.display_context_min_messages"


client_min_context. It's doing the same thing as min_messages does,
just for context instead of the message.

Or does this affect client and log the same way?


it affect client and log together

maybe "min_context"


+1
--
Jim Nasby, Data Architect, Blue Treble Consulting
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-04-27 22:53 GMT+02:00 Jim Nasby :

> On 4/27/15 11:47 AM, Joel Jacobson wrote:
>
>> On Mon, Apr 27, 2015 at 6:14 PM, Marko Tiikkaja > > wrote:
>>
>> That sounds weird.  log_min_messages are the messages sent to the
>> log; client_min_messages are sent to the client.
>> context_min_messages are not sent to a "context", whatever that
>> would mean.
>>
>>
>> Good point.
>>
>> I think it can't be any clearer than the proposed
>> "plpgsql.display_context_min_messages"
>>
>
> client_min_context. It's doing the same thing as min_messages does, just
> for context instead of the message.
>
> Or does this affect client and log the same way?


it affect client and log together

maybe "min_context"

Pavel


>
> --
> Jim Nasby, Data Architect, Blue Treble Consulting
> Data in Trouble? Get it in Treble! http://BlueTreble.com
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Jim Nasby]

On 4/27/15 11:47 AM, Joel Jacobson wrote:

On Mon, Apr 27, 2015 at 6:14 PM, Marko Tiikkaja mailto:ma...@joh.to>> wrote:

That sounds weird.  log_min_messages are the messages sent to the
log; client_min_messages are sent to the client.
context_min_messages are not sent to a "context", whatever that
would mean.


Good point.

I think it can't be any clearer than the proposed
"plpgsql.display_context_min_messages"


client_min_context. It's doing the same thing as min_messages does, just 
for context instead of the message.


Or does this affect client and log the same way?
--
Jim Nasby, Data Architect, Blue Treble Consulting
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] PL/pgSQL, RAISE and error context

[Joel Jacobson]

On Mon, Apr 27, 2015 at 6:14 PM, Marko Tiikkaja  wrote:

> That sounds weird.  log_min_messages are the messages sent to the log;
> client_min_messages are sent to the client.  context_min_messages are not
> sent to a "context", whatever that would mean.


Good point.

I think it can't be any clearer than the proposed
"plpgsql.display_context_min_messages"

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Marko Tiikkaja]

On 4/27/15 6:08 PM, Fabrízio de Royes Mello wrote:

On Sun, Apr 26, 2015 at 4:19 AM, Pavel Stehule 
wrote:

I reduced this patch, little bit cleaned - now it is based on plpgsql GUC

display_context_min_messages - like client_min_messages, log_min_messages.

What you think just "context_min_messages" ?


That sounds weird.  log_min_messages are the messages sent to the log; 
client_min_messages are sent to the client.  context_min_messages are 
not sent to a "context", whatever that would mean.



.m


--
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] PL/pgSQL, RAISE and error context

[Fabrízio de Royes Mello]

On Sun, Apr 26, 2015 at 4:19 AM, Pavel Stehule 
wrote:
>
> Hi
>
> I reduced this patch, little bit cleaned - now it is based on plpgsql GUC
display_context_min_messages - like client_min_messages, log_min_messages.
>

What you think just "context_min_messages" ?

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-04-27 16:05 GMT+02:00 Joel Jacobson :

> Looks good Pavel!
>
> May I just suggest you add the default case
> to src/test/regress/sql/plpgsql.sql
> and src/test/regress/expected/plpgsql.out, to make it easier for the
> reviewer to compare the difference between what happens in the default
> case, when not using the raise-syntax and not using the GUCs?
>
> Suggested addition to the beginning of src/test/regress/sql/plpgsql.sql:
> +do $$
> +begin
> +  raise notice 'hello';
> +end;
> +$$;
> +
> +do $$
> +begin
> +  raise exception 'hello';
> +end;
> +$$;
>

done


>
> Many thanks for this patch! I will pray to the PL/pgSQL God it will be
> accepted. :)
>

:) -- please, do review, or fix documentation in this patch.

I hope, so it will be merged early in 9.6 cycle. It is relatively simple.

Pavel


>
> Best regards,
>
> Joel
>
>
> On Sun, Apr 26, 2015 at 9:19 AM, Pavel Stehule 
> wrote:
>
>> Hi
>>
>> I reduced this patch, little bit cleaned - now it is based on plpgsql GUC
>> display_context_min_messages - like client_min_messages, log_min_messages.
>>
>> Documentation added.
>>
>> Regards
>>
>> Pavel
>>
>> 2015-04-25 22:23 GMT+02:00 Pavel Stehule :
>>
>>> Hi
>>>
>>> 2015-04-24 19:16 GMT+02:00 Joel Jacobson :
>>>
 On Fri, Apr 24, 2015 at 6:07 PM, Pavel Stehule 
 wrote:
 >> Example:
 >>
 >> context_messages = -warning, -error, +notice
 >
 >
 > I prefer your first proposal - and there is a precedent for plpgsql -
 > plpgsql_extra_checks
 >
 > It is clean for anybody. +-identifiers looks like horrible httpd
 config. :)

 I have to agree on that :) Just thought this is the best we can do if
 we want to reduce the number of GUCs to a minimum.

>>>
>>> I played with some prototype and I am thinking so we need only one GUC
>>>
>>> plpgsql.display_context_messages = 'none'; -- compatible with current
>>> plpgsql.display_context_messages = 'all';
>>> plpgsql.display_context_messages = 'exception, log'; -- what I prefer
>>>
>>> I implemented [ (WITH|WITHOUT) CONTEXT ] clause for RAISE statement
>>>
>>> RAISE NOTICE WITH CONTEXT 'some message';
>>> RAISE NOTICE WITH CONTEXT USING message = 'some message';
>>> RAISE EXCEPTION WITHOUT CONTEXT 'other message';
>>>
>>> The patch is very small with full functionality (without documentation)
>>> - I am thinking so it can work. This patch is back compatible - and allow
>>> to change default behave simply.
>>>
>>> plpgsql.display_context_messages can be simplified to some like
>>> plpgsql.display_context_min_messages
>>>
>>> What do you think about it?
>>>
>>> Regards
>>>
>>> Pavel
>>>
>>>
>>
>
commit d60c21fb798cf25609dc37a4bda3ec7822f790e1
Author: Pavel Stehule 
Date:   Sat Apr 25 22:09:28 2015 +0200

initial implementation of (WITH|WITHOUT) CONTEXT clause to plpgsql RAISE statement.

initial implementation of plpgsql GUC plpgsql.display_context_messages

diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index d36acf6..8aebb87 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -3406,10 +3406,10 @@ END LOOP  label ;
 raise errors.
 
 
-RAISE  level  'format' , expression , ...   USING option = expression , ...  ;
-RAISE  level  condition_name  USING option = expression , ...  ;
-RAISE  level  SQLSTATE 'sqlstate'  USING option = expression , ...  ;
-RAISE  level  USING option = expression , ... ;
+RAISE  level   ( WITH | WITHOUT ) CONTEXT  'format' , expression , ...   USING option = expression , ...  ;
+RAISE  level   ( WITH | WITHOUT ) CONTEXT  condition_name  USING option = expression , ...  ;
+RAISE  level   ( WITH | WITHOUT ) CONTEXT  SQLSTATE 'sqlstate'  USING option = expression , ...  ;
+RAISE  level   ( WITH | WITHOUT ) CONTEXT  USING option = expression , ... ;
 RAISE ;
 
 
@@ -3431,6 +3431,18 @@ RAISE ;

 

+The options WITH CONTEXT or WITHOUT CONTEXT
+can enforce or suppress context information related to error or notice. This possibility
+can be forced by settings of configuration parameter plpgsql.display_context_min_messages.
+This allows same values like level option plus
+value none that is a default. When it is changed, then all errors and notices
+with higher than specified severity are raised with context info.
+
+RAISE NOTICE WITH CONTEXT 'This message will have a context';
+
+   
+
+   
 After level if any,
 you can write a format
 (which must be a simple string literal, not an expression).  The
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index deefb1f..6d1e791 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2921,6 +2921,7 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
 	char	   *err_table = NULL;
 	char	   *err_schema = NULL;
 	ListCell   *lc;
+	bool			hide_ctx = true;		/* suppress context by default */
 
 	/* RAISE with no parameters: re-throw current exception */
 	if (stmt->condname == NULL && stmt->mes

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Joel Jacobson]

Looks good Pavel!

May I just suggest you add the default case
to src/test/regress/sql/plpgsql.sql
and src/test/regress/expected/plpgsql.out, to make it easier for the
reviewer to compare the difference between what happens in the default
case, when not using the raise-syntax and not using the GUCs?

Suggested addition to the beginning of src/test/regress/sql/plpgsql.sql:
+do $$
+begin
+  raise notice 'hello';
+end;
+$$;
+
+do $$
+begin
+  raise exception 'hello';
+end;
+$$;

Many thanks for this patch! I will pray to the PL/pgSQL God it will be
accepted. :)

Best regards,

Joel


On Sun, Apr 26, 2015 at 9:19 AM, Pavel Stehule 
wrote:

> Hi
>
> I reduced this patch, little bit cleaned - now it is based on plpgsql GUC
> display_context_min_messages - like client_min_messages, log_min_messages.
>
> Documentation added.
>
> Regards
>
> Pavel
>
> 2015-04-25 22:23 GMT+02:00 Pavel Stehule :
>
>> Hi
>>
>> 2015-04-24 19:16 GMT+02:00 Joel Jacobson :
>>
>>> On Fri, Apr 24, 2015 at 6:07 PM, Pavel Stehule 
>>> wrote:
>>> >> Example:
>>> >>
>>> >> context_messages = -warning, -error, +notice
>>> >
>>> >
>>> > I prefer your first proposal - and there is a precedent for plpgsql -
>>> > plpgsql_extra_checks
>>> >
>>> > It is clean for anybody. +-identifiers looks like horrible httpd
>>> config. :)
>>>
>>> I have to agree on that :) Just thought this is the best we can do if
>>> we want to reduce the number of GUCs to a minimum.
>>>
>>
>> I played with some prototype and I am thinking so we need only one GUC
>>
>> plpgsql.display_context_messages = 'none'; -- compatible with current
>> plpgsql.display_context_messages = 'all';
>> plpgsql.display_context_messages = 'exception, log'; -- what I prefer
>>
>> I implemented [ (WITH|WITHOUT) CONTEXT ] clause for RAISE statement
>>
>> RAISE NOTICE WITH CONTEXT 'some message';
>> RAISE NOTICE WITH CONTEXT USING message = 'some message';
>> RAISE EXCEPTION WITHOUT CONTEXT 'other message';
>>
>> The patch is very small with full functionality (without documentation) -
>> I am thinking so it can work. This patch is back compatible - and allow to
>> change default behave simply.
>>
>> plpgsql.display_context_messages can be simplified to some like
>> plpgsql.display_context_min_messages
>>
>> What do you think about it?
>>
>> Regards
>>
>> Pavel
>>
>>
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Pavel Stehule]

Hi

I reduced this patch, little bit cleaned - now it is based on plpgsql GUC
display_context_min_messages - like client_min_messages, log_min_messages.

Documentation added.

Regards

Pavel

2015-04-25 22:23 GMT+02:00 Pavel Stehule :

> Hi
>
> 2015-04-24 19:16 GMT+02:00 Joel Jacobson :
>
>> On Fri, Apr 24, 2015 at 6:07 PM, Pavel Stehule 
>> wrote:
>> >> Example:
>> >>
>> >> context_messages = -warning, -error, +notice
>> >
>> >
>> > I prefer your first proposal - and there is a precedent for plpgsql -
>> > plpgsql_extra_checks
>> >
>> > It is clean for anybody. +-identifiers looks like horrible httpd
>> config. :)
>>
>> I have to agree on that :) Just thought this is the best we can do if
>> we want to reduce the number of GUCs to a minimum.
>>
>
> I played with some prototype and I am thinking so we need only one GUC
>
> plpgsql.display_context_messages = 'none'; -- compatible with current
> plpgsql.display_context_messages = 'all';
> plpgsql.display_context_messages = 'exception, log'; -- what I prefer
>
> I implemented [ (WITH|WITHOUT) CONTEXT ] clause for RAISE statement
>
> RAISE NOTICE WITH CONTEXT 'some message';
> RAISE NOTICE WITH CONTEXT USING message = 'some message';
> RAISE EXCEPTION WITHOUT CONTEXT 'other message';
>
> The patch is very small with full functionality (without documentation) -
> I am thinking so it can work. This patch is back compatible - and allow to
> change default behave simply.
>
> plpgsql.display_context_messages can be simplified to some like
> plpgsql.display_context_min_messages
>
> What do you think about it?
>
> Regards
>
> Pavel
>
>
commit 33951bc23365029ee94af5ec43e90893dcd737a8
Author: Pavel Stehule 
Date:   Sat Apr 25 22:09:28 2015 +0200

initial implementation of (WITH|WITHOUT) CONTEXT clause to plpgsql RAISE statement.

initial implementation of plpgsql GUC plpgsql.display_context_messages

diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index d36acf6..8aebb87 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -3406,10 +3406,10 @@ END LOOP  label ;
 raise errors.
 
 
-RAISE  level  'format' , expression , ...   USING option = expression , ...  ;
-RAISE  level  condition_name  USING option = expression , ...  ;
-RAISE  level  SQLSTATE 'sqlstate'  USING option = expression , ...  ;
-RAISE  level  USING option = expression , ... ;
+RAISE  level   ( WITH | WITHOUT ) CONTEXT  'format' , expression , ...   USING option = expression , ...  ;
+RAISE  level   ( WITH | WITHOUT ) CONTEXT  condition_name  USING option = expression , ...  ;
+RAISE  level   ( WITH | WITHOUT ) CONTEXT  SQLSTATE 'sqlstate'  USING option = expression , ...  ;
+RAISE  level   ( WITH | WITHOUT ) CONTEXT  USING option = expression , ... ;
 RAISE ;
 
 
@@ -3431,6 +3431,18 @@ RAISE ;

 

+The options WITH CONTEXT or WITHOUT CONTEXT
+can enforce or suppress context information related to error or notice. This possibility
+can be forced by settings of configuration parameter plpgsql.display_context_min_messages.
+This allows same values like level option plus
+value none that is a default. When it is changed, then all errors and notices
+with higher than specified severity are raised with context info.
+
+RAISE NOTICE WITH CONTEXT 'This message will have a context';
+
+   
+
+   
 After level if any,
 you can write a format
 (which must be a simple string literal, not an expression).  The
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index deefb1f..6d1e791 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2921,6 +2921,7 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
 	char	   *err_table = NULL;
 	char	   *err_schema = NULL;
 	ListCell   *lc;
+	bool			hide_ctx = true;		/* suppress context by default */
 
 	/* RAISE with no parameters: re-throw current exception */
 	if (stmt->condname == NULL && stmt->message == NULL &&
@@ -3080,10 +3081,16 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
 			err_message = pstrdup(unpack_sql_state(err_code));
 	}
 
-	/*
-	 * Throw the error (may or may not come back)
-	 */
-	estate->err_text = raise_skip_msg;	/* suppress traceback of raise */
+	if (stmt->context_info == PLPGSQL_CONTEXT_DISPLAY)
+		hide_ctx = false;
+	else if (stmt->context_info == PLPGSQL_CONTEXT_DEFAULT)
+	{
+		if (plpgsql_display_context_min_messages != PLPGSQL_DISPLAY_CONTEXT_SUPPRESS)
+			hide_ctx = stmt->elog_level < plpgsql_display_context_min_messages;
+	}
+
+	if (hide_ctx)
+		estate->err_text = raise_skip_msg;
 
 	ereport(stmt->elog_level,
 			(err_code ? errcode(err_code) : 0,
@@ -3099,7 +3106,8 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
 			 (err_table != NULL) ?
 			 err_generic_string(PG_DIAG_TABLE_NAME, err_table) : 0,
 			 (err_schema != NULL) ?
-			 err_generic_string(PG_DIAG_SCHEMA_NAME, err_schema) : 0));
+			 err_generic_string(PG_DIAG_SCHEMA_NAME, e

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Joel Jacobson]

+1


On Sat, Apr 25, 2015 at 10:23 PM, Pavel Stehule 
wrote:

> Hi
>
> 2015-04-24 19:16 GMT+02:00 Joel Jacobson :
>
>> On Fri, Apr 24, 2015 at 6:07 PM, Pavel Stehule 
>> wrote:
>> >> Example:
>> >>
>> >> context_messages = -warning, -error, +notice
>> >
>> >
>> > I prefer your first proposal - and there is a precedent for plpgsql -
>> > plpgsql_extra_checks
>> >
>> > It is clean for anybody. +-identifiers looks like horrible httpd
>> config. :)
>>
>> I have to agree on that :) Just thought this is the best we can do if
>> we want to reduce the number of GUCs to a minimum.
>>
>
> I played with some prototype and I am thinking so we need only one GUC
>
> plpgsql.display_context_messages = 'none'; -- compatible with current
> plpgsql.display_context_messages = 'all';
> plpgsql.display_context_messages = 'exception, log'; -- what I prefer
>
> I implemented [ (WITH|WITHOUT) CONTEXT ] clause for RAISE statement
>
> RAISE NOTICE WITH CONTEXT 'some message';
> RAISE NOTICE WITH CONTEXT USING message = 'some message';
> RAISE EXCEPTION WITHOUT CONTEXT 'other message';
>
> The patch is very small with full functionality (without documentation) -
> I am thinking so it can work. This patch is back compatible - and allow to
> change default behave simply.
>
> plpgsql.display_context_messages can be simplified to some like
> plpgsql.display_context_min_messages
>
> What do you think about it?
>
> Regards
>
> Pavel
>
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Pavel Stehule]

Hi

2015-04-24 19:16 GMT+02:00 Joel Jacobson :

> On Fri, Apr 24, 2015 at 6:07 PM, Pavel Stehule 
> wrote:
> >> Example:
> >>
> >> context_messages = -warning, -error, +notice
> >
> >
> > I prefer your first proposal - and there is a precedent for plpgsql -
> > plpgsql_extra_checks
> >
> > It is clean for anybody. +-identifiers looks like horrible httpd config.
> :)
>
> I have to agree on that :) Just thought this is the best we can do if
> we want to reduce the number of GUCs to a minimum.
>

I played with some prototype and I am thinking so we need only one GUC

plpgsql.display_context_messages = 'none'; -- compatible with current
plpgsql.display_context_messages = 'all';
plpgsql.display_context_messages = 'exception, log'; -- what I prefer

I implemented [ (WITH|WITHOUT) CONTEXT ] clause for RAISE statement

RAISE NOTICE WITH CONTEXT 'some message';
RAISE NOTICE WITH CONTEXT USING message = 'some message';
RAISE EXCEPTION WITHOUT CONTEXT 'other message';

The patch is very small with full functionality (without documentation) - I
am thinking so it can work. This patch is back compatible - and allow to
change default behave simply.

plpgsql.display_context_messages can be simplified to some like
plpgsql.display_context_min_messages

What do you think about it?

Regards

Pavel
commit cf9e23a29162ac55fcab1ac4d9e7a24492de0736
Author: Pavel Stehule 
Date:   Sat Apr 25 22:09:28 2015 +0200

initial implementation of (WITH|WITHOUT) CONTEXT clause to plpgsql RAISE statement.

initial implementation of plpgsql GUC plpgsql.display_context_messages

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index deefb1f..ea0dac5 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2921,6 +2921,7 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
 	char	   *err_table = NULL;
 	char	   *err_schema = NULL;
 	ListCell   *lc;
+	bool			hide_ctx;
 
 	/* RAISE with no parameters: re-throw current exception */
 	if (stmt->condname == NULL && stmt->message == NULL &&
@@ -3080,10 +3081,42 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
 			err_message = pstrdup(unpack_sql_state(err_code));
 	}
 
-	/*
-	 * Throw the error (may or may not come back)
-	 */
-	estate->err_text = raise_skip_msg;	/* suppress traceback of raise */
+	if (stmt->context_info == PLPGSQL_CONTEXT_DISPLAY)
+		hide_ctx = false;
+	else if (stmt->context_info == PLPGSQL_CONTEXT_SUPPRESS)
+	{
+		hide_ctx = true;
+	}
+	else
+	{
+		/* we display a messages via plpgsql_display_context_messages */
+		switch (stmt->elog_level)
+		{
+			case ERROR:
+hide_ctx = !(plpgsql_display_context_messages & PLPGSQL_DISPLAY_CONTEXT_ERROR);
+break;
+			case WARNING:
+hide_ctx = !(plpgsql_display_context_messages & PLPGSQL_DISPLAY_CONTEXT_WARNING);
+break;
+			case NOTICE:
+hide_ctx = !(plpgsql_display_context_messages & PLPGSQL_DISPLAY_CONTEXT_NOTICE);
+break;
+			case INFO:
+hide_ctx = !(plpgsql_display_context_messages & PLPGSQL_DISPLAY_CONTEXT_INFO);
+break;
+			case LOG:
+hide_ctx = !(plpgsql_display_context_messages & PLPGSQL_DISPLAY_CONTEXT_LOG);
+break;
+			case DEBUG1:
+hide_ctx = !(plpgsql_display_context_messages & PLPGSQL_DISPLAY_CONTEXT_DEBUG);
+break;
+			default:
+elog(ERROR, "unexpected RAISE statement level");
+		}
+	}
+
+	if (hide_ctx)
+		estate->err_text = raise_skip_msg;
 
 	ereport(stmt->elog_level,
 			(err_code ? errcode(err_code) : 0,
@@ -3099,7 +3132,8 @@ exec_stmt_raise(PLpgSQL_execstate *estate, PLpgSQL_stmt_raise *stmt)
 			 (err_table != NULL) ?
 			 err_generic_string(PG_DIAG_TABLE_NAME, err_table) : 0,
 			 (err_schema != NULL) ?
-			 err_generic_string(PG_DIAG_SCHEMA_NAME, err_schema) : 0));
+			 err_generic_string(PG_DIAG_SCHEMA_NAME, err_schema) : 0,
+			 errhidecontext(hide_ctx)));
 
 	estate->err_text = NULL;	/* un-suppress... */
 
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 4026e41..48914a7 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -259,6 +259,7 @@ static	void			check_raise_parameters(PLpgSQL_stmt_raise *stmt);
 %token 	K_CONSTANT
 %token 	K_CONSTRAINT
 %token 	K_CONSTRAINT_NAME
+%token 	K_CONTEXT
 %token 	K_CONTINUE
 %token 	K_CURRENT
 %token 	K_CURSOR
@@ -341,6 +342,8 @@ static	void			check_raise_parameters(PLpgSQL_stmt_raise *stmt);
 %token 	K_WARNING
 %token 	K_WHEN
 %token 	K_WHILE
+%token 	K_WITH
+%token 	K_WITHOUT
 
 %%
 
@@ -1716,6 +1719,7 @@ stmt_raise		: K_RAISE
 		new->cmd_type	= PLPGSQL_STMT_RAISE;
 		new->lineno		= plpgsql_location_to_lineno(@1);
 		new->elog_level = ERROR;	/* default */
+		new->context_info = PLPGSQL_CONTEXT_DEFAULT;
 		new->condname	= NULL;
 		new->message	= NULL;
 		new->params		= NIL;
@@ -1773,6 +1777,21 @@ stmt_raise		: K_RAISE
 			if (tok == 0)
 yyerror("unexpected end of function definition");
 
+			/* Optional choose about including context */

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-04-24 19:16 GMT+02:00 Joel Jacobson :

> On Fri, Apr 24, 2015 at 6:07 PM, Pavel Stehule 
> wrote:
> >> Example:
> >>
> >> context_messages = -warning, -error, +notice
> >
> >
> > I prefer your first proposal - and there is a precedent for plpgsql -
> > plpgsql_extra_checks
> >
> > It is clean for anybody. +-identifiers looks like horrible httpd config.
> :)
>
> I have to agree on that :) Just thought this is the best we can do if
> we want to reduce the number of GUCs to a minimum.
>

It looks like discussion KDE x GNOME.

GUC that has simply effect on behave without performance impact should not
be problem - like log_lock_wait, log_min_duration and similar. I am sure so
we would it.

The problematic GUC are a performance, planner, bgwriter, checkpoint
related.

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Joel Jacobson]

On Fri, Apr 24, 2015 at 6:07 PM, Pavel Stehule  wrote:
>> Example:
>>
>> context_messages = -warning, -error, +notice
>
>
> I prefer your first proposal - and there is a precedent for plpgsql -
> plpgsql_extra_checks
>
> It is clean for anybody. +-identifiers looks like horrible httpd config. :)

I have to agree on that :) Just thought this is the best we can do if
we want to reduce the number of GUCs to a minimum.


-- 
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-04-24 16:02 GMT+02:00 Joel Jacobson :

> On Fri, Apr 24, 2015 at 1:16 PM, Robert Haas 
> wrote:
> >> This would allow doing something crazy as:
> >>
> >> suppress_context_messages = warning,error
> >> display_context_messages = notice
> >
> > This is a very flexible proposal, but it's a tremendous amount of
> > machinery for what's really a very minor issue.  If we added two GUCs
> > for every comparably important issue, we'd have about 40,000 of them.
>
> I agree. The one-dimensional GUC syntax is not well suited for
> multi-dimensional config settings. And that's a good thing mostly I
> think. It would be a nightmare if the config file values could in JSON
> format, it's good they are simple.
>
> But I'm thinking maybe we could improve the config file syntax for the
> general case when you have multiple things you want to control, in
> this case the message levels, and for each such thing, you want to
> turn something on/off, in this case the CONTEXT. Maybe we could simply
> use plus "+" and minus "-" to mean "on" and "off"?
>
> Example:
>
> context_messages = -warning, -error, +notice
>

I prefer your first proposal - and there is a precedent for plpgsql -
plpgsql_extra_checks

It is clean for anybody. +-identifiers looks like horrible httpd config. :)

Regards

Pavel

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Joel Jacobson]

On Fri, Apr 24, 2015 at 1:16 PM, Robert Haas  wrote:
>> This would allow doing something crazy as:
>>
>> suppress_context_messages = warning,error
>> display_context_messages = notice
>
> This is a very flexible proposal, but it's a tremendous amount of
> machinery for what's really a very minor issue.  If we added two GUCs
> for every comparably important issue, we'd have about 40,000 of them.

I agree. The one-dimensional GUC syntax is not well suited for
multi-dimensional config settings. And that's a good thing mostly I
think. It would be a nightmare if the config file values could in JSON
format, it's good they are simple.

But I'm thinking maybe we could improve the config file syntax for the
general case when you have multiple things you want to control, in
this case the message levels, and for each such thing, you want to
turn something on/off, in this case the CONTEXT. Maybe we could simply
use plus "+" and minus "-" to mean "on" and "off"?

Example:

context_messages = -warning, -error, +notice


-- 
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-04-24 13:16 GMT+02:00 Robert Haas :

> On Fri, Apr 24, 2015 at 6:11 AM, Joel Jacobson  wrote:
> > Entering the discussion because this is a huge pain for me in my daily
> > work as well.
> >
> > This is not a reply to any specific post in this thread, but my first
> > message in the thread.
> >
> > I see a great value in providing both a GUC and a new RAISE syntax.
> > The different benefits of the two are maybe obvious, but perhaps worth
> > pointing out:
> > GUC: Good because you don't have to change any existing code.
> > RAISE syntax: Good because you can control exactly what message should
> > be emitted or not be emitted at that line of code.
> >
> > I think preserving backwards compatibility is very important.
> > Not changing the default is not a problem for me, as long as it can be
> > overridden.
> >
> > Whatever the default behaviour is, I think the need expressed by all
> > users in this thread boils down to any of these two sentences:
> >
> > "I want CONTEXT to be (DISPLAYED|SUPPRESSED) for (ALL|ONLY THIS LINE)
> > RAISE (NOTICE|WARNING|ERROR)"
> > OR
> > "I don't want to change the default current behaviour of CONTEXT"
> >
> > So we basically need a boolean setting value, where:
> > NULL means the default behaviour
> > TRUE means DISPLAY CONTEXT
> > FALSE means SUPPRESS CONTEXT
> >
> > And the (ALL|ONLY THIS) part translates into using,
> > * a GUC to change behaviour for ALL lines of code,
> > * or using the RAISE syntax to change the behaviour of ONLY THIS line of
> code.
> >
> > And then we have the different message levels, for which CONTEXT is
> > sometimes desirable in some situations:
> > * The RAISE syntax allows controlling any message level in a natural
> > way, as the message level is part of the syntax.
> > * Allowing the same control using GUC would mean the message level
> > would need to be part of the GUC key name, which means either add
> > multiple GUCs, one for each message level, or only allow controlling
> > the most important one and ignore the possibly need to control the
> > other message levels.
> >
> > If it would be possible to somehow combine multiple message levels in
> > the same GUC, that would solve the latter problem.
> >
> > We already have comma separated values for many GUCs, so maybe we
> > could use that approach here as well.
> >
> > It looks like adding these two GUCs would meet the demands of all users:
> >
> > suppress_context_messages (enum)
> > display_context_messages (enum)
> >
> > This would allow doing something crazy as:
> >
> > suppress_context_messages = warning,error
> > display_context_messages = notice
>
> This is a very flexible proposal, but it's a tremendous amount of
> machinery for what's really a very minor issue.  If we added two GUCs
> for every comparably important issue, we'd have about 40,000 of them.
>

It can be PLpgSQL only GUC. Probably it is a most problematic case.

>
> I suggest we add the RAISE syntax first, because everybody agrees on
> that.  Then, we can argue about the other stuff.
>

There is a agreement about it - but I am expecting a harder discussion
about what will be default, and the discussion about syntax should not be
simple too.




>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Robert Haas]

On Fri, Apr 24, 2015 at 6:11 AM, Joel Jacobson  wrote:
> Entering the discussion because this is a huge pain for me in my daily
> work as well.
>
> This is not a reply to any specific post in this thread, but my first
> message in the thread.
>
> I see a great value in providing both a GUC and a new RAISE syntax.
> The different benefits of the two are maybe obvious, but perhaps worth
> pointing out:
> GUC: Good because you don't have to change any existing code.
> RAISE syntax: Good because you can control exactly what message should
> be emitted or not be emitted at that line of code.
>
> I think preserving backwards compatibility is very important.
> Not changing the default is not a problem for me, as long as it can be
> overridden.
>
> Whatever the default behaviour is, I think the need expressed by all
> users in this thread boils down to any of these two sentences:
>
> "I want CONTEXT to be (DISPLAYED|SUPPRESSED) for (ALL|ONLY THIS LINE)
> RAISE (NOTICE|WARNING|ERROR)"
> OR
> "I don't want to change the default current behaviour of CONTEXT"
>
> So we basically need a boolean setting value, where:
> NULL means the default behaviour
> TRUE means DISPLAY CONTEXT
> FALSE means SUPPRESS CONTEXT
>
> And the (ALL|ONLY THIS) part translates into using,
> * a GUC to change behaviour for ALL lines of code,
> * or using the RAISE syntax to change the behaviour of ONLY THIS line of code.
>
> And then we have the different message levels, for which CONTEXT is
> sometimes desirable in some situations:
> * The RAISE syntax allows controlling any message level in a natural
> way, as the message level is part of the syntax.
> * Allowing the same control using GUC would mean the message level
> would need to be part of the GUC key name, which means either add
> multiple GUCs, one for each message level, or only allow controlling
> the most important one and ignore the possibly need to control the
> other message levels.
>
> If it would be possible to somehow combine multiple message levels in
> the same GUC, that would solve the latter problem.
>
> We already have comma separated values for many GUCs, so maybe we
> could use that approach here as well.
>
> It looks like adding these two GUCs would meet the demands of all users:
>
> suppress_context_messages (enum)
> display_context_messages (enum)
>
> This would allow doing something crazy as:
>
> suppress_context_messages = warning,error
> display_context_messages = notice

This is a very flexible proposal, but it's a tremendous amount of
machinery for what's really a very minor issue.  If we added two GUCs
for every comparably important issue, we'd have about 40,000 of them.

I suggest we add the RAISE syntax first, because everybody agrees on
that.  Then, we can argue about the other stuff.

-- 
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-04-24 12:11 GMT+02:00 Joel Jacobson :

> Entering the discussion because this is a huge pain for me in my daily
> work as well.
>
> This is not a reply to any specific post in this thread, but my first
> message in the thread.
>
> I see a great value in providing both a GUC and a new RAISE syntax.
> The different benefits of the two are maybe obvious, but perhaps worth
> pointing out:
> GUC: Good because you don't have to change any existing code.
> RAISE syntax: Good because you can control exactly what message should
> be emitted or not be emitted at that line of code.
>
> I think preserving backwards compatibility is very important.
> Not changing the default is not a problem for me, as long as it can be
> overridden.
>
> Whatever the default behaviour is, I think the need expressed by all
> users in this thread boils down to any of these two sentences:
>
> "I want CONTEXT to be (DISPLAYED|SUPPRESSED) for (ALL|ONLY THIS LINE)
> RAISE (NOTICE|WARNING|ERROR)"
> OR
> "I don't want to change the default current behaviour of CONTEXT"
>
> So we basically need a boolean setting value, where:
> NULL means the default behaviour
> TRUE means DISPLAY CONTEXT
> FALSE means SUPPRESS CONTEXT
>
> And the (ALL|ONLY THIS) part translates into using,
> * a GUC to change behaviour for ALL lines of code,
> * or using the RAISE syntax to change the behaviour of ONLY THIS line of
> code.
>
> And then we have the different message levels, for which CONTEXT is
> sometimes desirable in some situations:
> * The RAISE syntax allows controlling any message level in a natural
> way, as the message level is part of the syntax.
> * Allowing the same control using GUC would mean the message level
> would need to be part of the GUC key name, which means either add
> multiple GUCs, one for each message level, or only allow controlling
> the most important one and ignore the possibly need to control the
> other message levels.
>
> If it would be possible to somehow combine multiple message levels in
> the same GUC, that would solve the latter problem.
>
> We already have comma separated values for many GUCs, so maybe we
> could use that approach here as well.
>
> It looks like adding these two GUCs would meet the demands of all users:
>
>
>

> suppress_context_messages (enum)
> display_context_messages (enum)
>
<

This proposal looks very practical - it can be very good start point - and
it doesn't block any next discuss about enhancing RAISE statement, what I
would to have too (bat can be separate issue). I like it.

Regards

Pavel



>
> This would allow doing something crazy as:
>
> suppress_context_messages = warning,error
> display_context_messages = notice
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Joel Jacobson]

Entering the discussion because this is a huge pain for me in my daily
work as well.

This is not a reply to any specific post in this thread, but my first
message in the thread.

I see a great value in providing both a GUC and a new RAISE syntax.
The different benefits of the two are maybe obvious, but perhaps worth
pointing out:
GUC: Good because you don't have to change any existing code.
RAISE syntax: Good because you can control exactly what message should
be emitted or not be emitted at that line of code.

I think preserving backwards compatibility is very important.
Not changing the default is not a problem for me, as long as it can be
overridden.

Whatever the default behaviour is, I think the need expressed by all
users in this thread boils down to any of these two sentences:

"I want CONTEXT to be (DISPLAYED|SUPPRESSED) for (ALL|ONLY THIS LINE)
RAISE (NOTICE|WARNING|ERROR)"
OR
"I don't want to change the default current behaviour of CONTEXT"

So we basically need a boolean setting value, where:
NULL means the default behaviour
TRUE means DISPLAY CONTEXT
FALSE means SUPPRESS CONTEXT

And the (ALL|ONLY THIS) part translates into using,
* a GUC to change behaviour for ALL lines of code,
* or using the RAISE syntax to change the behaviour of ONLY THIS line of code.

And then we have the different message levels, for which CONTEXT is
sometimes desirable in some situations:
* The RAISE syntax allows controlling any message level in a natural
way, as the message level is part of the syntax.
* Allowing the same control using GUC would mean the message level
would need to be part of the GUC key name, which means either add
multiple GUCs, one for each message level, or only allow controlling
the most important one and ignore the possibly need to control the
other message levels.

If it would be possible to somehow combine multiple message levels in
the same GUC, that would solve the latter problem.

We already have comma separated values for many GUCs, so maybe we
could use that approach here as well.

It looks like adding these two GUCs would meet the demands of all users:

suppress_context_messages (enum)
display_context_messages (enum)

This would allow doing something crazy as:

suppress_context_messages = warning,error
display_context_messages = notice


-- 
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-04-23 16:12 GMT+02:00 Robert Haas :

> On Thu, Apr 23, 2015 at 9:55 AM, Pavel Stehule 
> wrote:
> >> On Thu, Apr 23, 2015 at 4:56 AM, Pavel Stehule  >
> >> wrote:
> >> > I don't see a contradiction. There is clean agreement, so ERROR level
> >> > should
> >> > to show the context. NOTICE and WARNINGs doesn't need it - and there
> is
> >> > a
> >> > backward compatibility and usability reasons don't do it.
> >>
> >> Whether notices and warnings need it is a matter of opinion.  I don't
> >> think your idea is bad, and it might be a good rule of thumb in many
> >> cases, but I slightly prefer Marko's approach of adding a new option.
> >
> > I am not sure if I understand to you.
> >
> > please, can you write more about your idea?
>
> Your idea, as I understand it, is that for logs at severity levels
> lower than ERROR, we can always emit the context, because it's not
> necessary.  But I'm not sure that's right: some people might find that
> context helpful.  If, as Marko proposes, we add an explicit option,
> then everyone can choose the behavior that is right for them.
>

I am not sure, if explained it well. I would to emit context for ERROR and
higher by default. And I would not to emit context for any less than ERROR
by default (I am not sure about WARNING level).

But it can be changed by some option in RAISE statement like Marko proposes
- possible to change by GUC globally, because it doesn't change a behave of
application.

For current behave I have a problem with ERROR level in plpgsql where the
context is missing now.  On second hand I am thinking so current behave is
ok for NOTICE level .

I am not against to any new option in RAISE statement.

If there is some collision between me and Marko, then it is in opinion what
have to be default behave for NOTICE level. I strongly prefer don't show
context there. But I can accept some global switch too.

Regards

Pavel


>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Robert Haas]

On Thu, Apr 23, 2015 at 9:55 AM, Pavel Stehule  wrote:
>> On Thu, Apr 23, 2015 at 4:56 AM, Pavel Stehule 
>> wrote:
>> > I don't see a contradiction. There is clean agreement, so ERROR level
>> > should
>> > to show the context. NOTICE and WARNINGs doesn't need it - and there is
>> > a
>> > backward compatibility and usability reasons don't do it.
>>
>> Whether notices and warnings need it is a matter of opinion.  I don't
>> think your idea is bad, and it might be a good rule of thumb in many
>> cases, but I slightly prefer Marko's approach of adding a new option.
>
> I am not sure if I understand to you.
>
> please, can you write more about your idea?

Your idea, as I understand it, is that for logs at severity levels
lower than ERROR, we can always emit the context, because it's not
necessary.  But I'm not sure that's right: some people might find that
context helpful.  If, as Marko proposes, we add an explicit option,
then everyone can choose the behavior that is right for them.

-- 
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-04-23 15:47 GMT+02:00 Robert Haas :

> On Thu, Apr 23, 2015 at 4:56 AM, Pavel Stehule 
> wrote:
> > I don't see a contradiction. There is clean agreement, so ERROR level
> should
> > to show the context. NOTICE and WARNINGs doesn't need it - and there is a
> > backward compatibility and usability reasons don't do it.
>
> Whether notices and warnings need it is a matter of opinion.  I don't
> think your idea is bad, and it might be a good rule of thumb in many
> cases, but I slightly prefer Marko's approach of adding a new option.
>

I am not sure if I understand to you.

please, can you write more about your idea?




>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Robert Haas]

On Thu, Apr 23, 2015 at 4:56 AM, Pavel Stehule  wrote:
> I don't see a contradiction. There is clean agreement, so ERROR level should
> to show the context. NOTICE and WARNINGs doesn't need it - and there is a
> backward compatibility and usability reasons don't do it.

Whether notices and warnings need it is a matter of opinion.  I don't
think your idea is bad, and it might be a good rule of thumb in many
cases, but I slightly prefer Marko's approach of adding a new option.

-- 
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-04-23 9:53 GMT+02:00 Marko Tiikkaja :

> On 4/2/15 9:37 AM, Pavel Stehule wrote:
>
>> estate->err_text = stmt->elog_level == ERROR ? estate->err_text :
>>> raise_skip_msg  ;
>>>
>>
>> Can we do this simple change? It will produce a stackinfo for exceptions
>> and it will not to make mad developers by lot of useless content.
>>
>
> I'm not sure everyone agrees with this to be honest, myself included.
>
> I think the best way to do this would be to have an option for RAISE to
> suppress the context *regardless of nesting depth*, but show the full
> context by default for ERRORs.  For NOTICEs and WARNINGs I don't care much
> what the default will be; perhaps just full backwards compatibility could
> work there.
>

I don't see a contradiction. There is clean agreement, so ERROR level
should to show the context. NOTICE and WARNINGs doesn't need it - and there
is a backward compatibility and usability reasons don't do it.

I am not to against to any special option to RAISE statement. Have you some
idea?

What about a enhancing a USING clause?

example:

RAISE NOTICE USING message = '', with_context = true
RAISE EXCEPTION USING message = '', with_context = false


Regards

Pavel



>
>
> .m
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Marko Tiikkaja]

On 4/2/15 9:37 AM, Pavel Stehule wrote:

estate->err_text = stmt->elog_level == ERROR ? estate->err_text :
raise_skip_msg  ;


Can we do this simple change? It will produce a stackinfo for exceptions
and it will not to make mad developers by lot of useless content.


I'm not sure everyone agrees with this to be honest, myself included.

I think the best way to do this would be to have an option for RAISE to 
suppress the context *regardless of nesting depth*, but show the full 
context by default for ERRORs.  For NOTICEs and WARNINGs I don't care 
much what the default will be; perhaps just full backwards compatibility 
could work there.



.m


--
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-01-26 16:46 GMT+01:00 Pavel Stehule :

>
>
> 2015-01-26 16:14 GMT+01:00 Tom Lane :
>
>> Pavel Stehule  writes:
>> > 2015-01-26 14:02 GMT+01:00 Marko Tiikkaja :
>> > I am thinking, so solution
>>
>> >  /* if we are doing RAISE, don't report its location */
>> > if (estate->err_text == raise_skip_msg)
>> > return;
>>
>> > is too simple, and this part should be fixed. This change can be done
>> by on
>> > plpgsql or libpq side. This is bug, and it should be fixed.
>>
>> Doing this in libpq is utterly insane.  It has not got sufficient context
>> to do anything intelligent.  The fact that it's not intelligent is exposed
>> by the regression test changes that the proposed patch causes, most of
>> which do not look like improvements.
>>
>>
> I don't understand. There can be a overhead due useless transformation
> some data to client side. But all what it need - errcontext and errlevel is
> possible.
>
>
>> Another problem is that past requests to change this behavior have
>> generally been to the effect that people wanted *more* context suppressed
>> not less, ie they didn't want any CONTEXT lines at all on certain
>> messages.  So the proposed patch seems to me to be going in exactly the
>> wrong direction.
>>
>> The design I thought had been agreed on was to add some new option to
>> plpgsql's RAISE command which would cause suppression of all CONTEXT lines
>> not just the most closely nested one.  You could argue about whether the
>> behavior needs to be 100% backwards compatible or not --- if so, perhaps
>> it could be a three-way option all, none, or one line, defaulting to the
>> last for backwards compatibility.
>>
>
>  I see a problem what should be default behave. When I raise NOTICE, then
> I don't need (don't would) to see CONTEXT lines, When I raise EXCEPTION,
> then I usually would to see CONTEXT lines.
>
> Cannot be solution?
>

I would to wakeup this thread.


>
> estate->err_text = stmt->elog_level == ERROR ? estate->err_text :
> raise_skip_msg  ;
>

Can we do this simple change? It will produce a stackinfo for exceptions
and it will not to make mad developers by lot of useless content.

Regards

Pavel


>
> Regards
>
> Pavel
>
>
>
>
>>
>> regards, tom lane
>>
>
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Jim Nasby]

On 1/26/15 9:46 AM, Pavel Stehule wrote:


The design I thought had been agreed on was to add some new option to
plpgsql's RAISE command which would cause suppression of all CONTEXT lines
not just the most closely nested one.  You could argue about whether the
behavior needs to be 100% backwards compatible or not --- if so, perhaps
it could be a three-way option all, none, or one line, defaulting to the
last for backwards compatibility.


  I see a problem what should be default behave. When I raise NOTICE, then I 
don't need (don't would) to see CONTEXT lines, When I raise EXCEPTION, then I 
usually would to see CONTEXT lines.


FWIW, that's the case I almost always run into: I turn on some debugging which 
means I know where the RAISE is coming from, but now I'm flooded with CONTEXT 
lines. You could do that with an option to RAISE, but that seems like a lot of 
extra coding work for little gain. Perhaps it'd be worth creating 
client_min_context and log_min_context GUCs...

Another option that I think would work well is that you only provide context for the 
first call within a "block" of code. For plpgsql that would be a function, but 
maybe it'd be better to just do this per-subtransaction.

I do agree that this needs to work across the board, not just for plpgsql.
--
Jim Nasby, Data Architect, Blue Treble Consulting
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-01-26 16:14 GMT+01:00 Tom Lane :

> Pavel Stehule  writes:
> > 2015-01-26 14:02 GMT+01:00 Marko Tiikkaja :
> > I am thinking, so solution
>
> >  /* if we are doing RAISE, don't report its location */
> > if (estate->err_text == raise_skip_msg)
> > return;
>
> > is too simple, and this part should be fixed. This change can be done by
> on
> > plpgsql or libpq side. This is bug, and it should be fixed.
>
> Doing this in libpq is utterly insane.  It has not got sufficient context
> to do anything intelligent.  The fact that it's not intelligent is exposed
> by the regression test changes that the proposed patch causes, most of
> which do not look like improvements.
>
>
I don't understand. There can be a overhead due useless transformation some
data to client side. But all what it need - errcontext and errlevel is
possible.


> Another problem is that past requests to change this behavior have
> generally been to the effect that people wanted *more* context suppressed
> not less, ie they didn't want any CONTEXT lines at all on certain
> messages.  So the proposed patch seems to me to be going in exactly the
> wrong direction.
>
> The design I thought had been agreed on was to add some new option to
> plpgsql's RAISE command which would cause suppression of all CONTEXT lines
> not just the most closely nested one.  You could argue about whether the
> behavior needs to be 100% backwards compatible or not --- if so, perhaps
> it could be a three-way option all, none, or one line, defaulting to the
> last for backwards compatibility.
>

 I see a problem what should be default behave. When I raise NOTICE, then I
don't need (don't would) to see CONTEXT lines, When I raise EXCEPTION, then
I usually would to see CONTEXT lines.

Cannot be solution?

estate->err_text = stmt->elog_level == ERROR ? estate->err_text :
raise_skip_msg  ;

Regards

Pavel




>
> regards, tom lane
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Tom Lane]

Pavel Stehule  writes:
> 2015-01-26 14:02 GMT+01:00 Marko Tiikkaja :
> I am thinking, so solution

>  /* if we are doing RAISE, don't report its location */
> if (estate->err_text == raise_skip_msg)
> return;

> is too simple, and this part should be fixed. This change can be done by on
> plpgsql or libpq side. This is bug, and it should be fixed.

Doing this in libpq is utterly insane.  It has not got sufficient context
to do anything intelligent.  The fact that it's not intelligent is exposed
by the regression test changes that the proposed patch causes, most of
which do not look like improvements.

Another problem is that past requests to change this behavior have
generally been to the effect that people wanted *more* context suppressed
not less, ie they didn't want any CONTEXT lines at all on certain
messages.  So the proposed patch seems to me to be going in exactly the
wrong direction.

The design I thought had been agreed on was to add some new option to
plpgsql's RAISE command which would cause suppression of all CONTEXT lines
not just the most closely nested one.  You could argue about whether the
behavior needs to be 100% backwards compatible or not --- if so, perhaps
it could be a three-way option all, none, or one line, defaulting to the
last for backwards compatibility.

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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-01-26 14:02 GMT+01:00 Marko Tiikkaja :

> On 1/26/15 1:44 PM, Pavel Stehule wrote:
>
>> 2015-01-26 13:39 GMT+01:00 Marko Tiikkaja :
>>
>>> On 1/26/15 1:14 PM, Pavel Stehule wrote:
>>>
 I afraid about some unexpected side effects of your proposal if somebody
 mix languages - these side effects should not be critical


>>> I have no idea what you're talking about.  What kind of side effects?
>>>
>>>
>> what will be a error context if plpgsql calls a plperl function that
>> raises
>> a exception
>> what will be a error context if plperl calls a plpgsql functions that
>> raises a exception
>>
>
> I fail to see the point.  How would that be different from what happens
> today?  Remember, PL/PgSQL only suppresses the *topmost* stack frame, and
> only when using RAISE from within a PL/PgSQL function.
>

I had to though little bit more - and I am thinking so we should to return
back to start of this thread.

Current state:

1. RAISE in plpgsql doesn't show a context - what we want in RAISE NOTICE
and we don't want in RAISE EXCEPTION

I am thinking, so solution

 /* if we are doing RAISE, don't report its location */
if (estate->err_text == raise_skip_msg)
return;

is too simple, and this part should be fixed. This change can be done by on
plpgsql or libpq side. This is bug, and it should be fixed.

2. Personally I prefer a little bit conceptual solution, that needs a libpq
change because I wish some mode between terse and verbose mode - I would
not to see context for NOTICEs, but I would to see context for errors. This
request is generic - independent on used PL. @2 is my feature request and
it is possible independent on @1.

3. your proposal plpgsql.suppress_simple_error_context fix the @2 partially
- just I prefer a generic solution that will be available for all PL -
exception processing is same for all PL, so filtering should be common too.

Regards

Pavel


>
>
> .m
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Marko Tiikkaja]

On 1/26/15 1:44 PM, Pavel Stehule wrote:

2015-01-26 13:39 GMT+01:00 Marko Tiikkaja :

On 1/26/15 1:14 PM, Pavel Stehule wrote:

I afraid about some unexpected side effects of your proposal if somebody
mix languages - these side effects should not be critical



I have no idea what you're talking about.  What kind of side effects?



what will be a error context if plpgsql calls a plperl function that raises
a exception
what will be a error context if plperl calls a plpgsql functions that
raises a exception


I fail to see the point.  How would that be different from what happens 
today?  Remember, PL/PgSQL only suppresses the *topmost* stack frame, 
and only when using RAISE from within a PL/PgSQL function.



.m


--
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-01-26 13:39 GMT+01:00 Marko Tiikkaja :

> On 1/26/15 1:14 PM, Pavel Stehule wrote:
>
>> 2015-01-26 13:02 GMT+01:00 Marko Tiikkaja :
>>
>>> I can see where it's a lot nicer not to have the context visible for
>>> people who only care about the contents of the message, but the way it's
>>> done in PL/PgSQL right now is just not good enough.  On the other hand,
>>> the
>>> backwards compatibility breakage of doing this in libpq is quite
>>> extensive.  The most simple option seems to be to just allow a GUC to
>>> change PL/PgSQL's behavior to match what all other PLs are doing.
>>>
>>>
>> libpq was changed more time - there is still a open task about a protocol
>> change.
>>
>> I afraid about some unexpected side effects of your proposal if somebody
>> mix languages - these side effects should not be critical
>>
>
> I have no idea what you're talking about.  What kind of side effects?
>

what will be a error context if plpgsql calls a plperl function that raises
a exception
what will be a error context if plperl calls a plpgsql functions that
raises a exception

>
>  - but on second
>> hand current behave is not critical too - we can wait.
>>
>
> I think the current behavior is almost unacceptable.  It makes debugging
> in some cases really, really difficult.
>

if it is necessary, then we can modify libpq

Regards

Pavel



>
>
> .marko
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Marko Tiikkaja]

On 1/26/15 1:14 PM, Pavel Stehule wrote:

2015-01-26 13:02 GMT+01:00 Marko Tiikkaja :

I can see where it's a lot nicer not to have the context visible for
people who only care about the contents of the message, but the way it's
done in PL/PgSQL right now is just not good enough.  On the other hand, the
backwards compatibility breakage of doing this in libpq is quite
extensive.  The most simple option seems to be to just allow a GUC to
change PL/PgSQL's behavior to match what all other PLs are doing.



libpq was changed more time - there is still a open task about a protocol
change.

I afraid about some unexpected side effects of your proposal if somebody
mix languages - these side effects should not be critical


I have no idea what you're talking about.  What kind of side effects?


- but on second
hand current behave is not critical too - we can wait.


I think the current behavior is almost unacceptable.  It makes debugging 
in some cases really, really difficult.



.marko


--
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-01-26 13:02 GMT+01:00 Marko Tiikkaja :

> On 1/22/15 6:03 PM, Pavel Stehule wrote:
>
>> 2015-01-22 12:37 GMT+01:00 Marko Tiikkaja :
>>
>>> Or is that a stupid idea?  I just think hacking libpq for something like
>>> this is a huge overkill.
>>>
>>>
>> I don't think so only plpgsql  solution is satisfactory idea. There are
>> some mix plpgsql / plperl ... application - and it isn't possible to
>> remove
>> error context from only one language.
>>
>
> Yeah, not in libpq it isn't.  Thing is, PL/PgSQL already is the exception
> here, since it's the only language which does this error message
> suppression.  So if people did think this suppression was a good idea, only
> the people using PL/PgSQL were vocal enough to get the behavior changed.
> I'm not looking to change that.
>

> I can see where it's a lot nicer not to have the context visible for
> people who only care about the contents of the message, but the way it's
> done in PL/PgSQL right now is just not good enough.  On the other hand, the
> backwards compatibility breakage of doing this in libpq is quite
> extensive.  The most simple option seems to be to just allow a GUC to
> change PL/PgSQL's behavior to match what all other PLs are doing.
>


libpq was changed more time - there is still a open task about a protocol
change.

I afraid about some unexpected side effects of your proposal if somebody
mix languages - these side effects should not be critical - but on second
hand current behave is not critical too - we can wait.


>
>
> .marko
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Marko Tiikkaja]

On 1/22/15 6:03 PM, Pavel Stehule wrote:

2015-01-22 12:37 GMT+01:00 Marko Tiikkaja :

Or is that a stupid idea?  I just think hacking libpq for something like
this is a huge overkill.



I don't think so only plpgsql  solution is satisfactory idea. There are
some mix plpgsql / plperl ... application - and it isn't possible to remove
error context from only one language.


Yeah, not in libpq it isn't.  Thing is, PL/PgSQL already is the 
exception here, since it's the only language which does this error 
message suppression.  So if people did think this suppression was a good 
idea, only the people using PL/PgSQL were vocal enough to get the 
behavior changed.  I'm not looking to change that.


I can see where it's a lot nicer not to have the context visible for 
people who only care about the contents of the message, but the way it's 
done in PL/PgSQL right now is just not good enough.  On the other hand, 
the backwards compatibility breakage of doing this in libpq is quite 
extensive.  The most simple option seems to be to just allow a GUC to 
change PL/PgSQL's behavior to match what all other PLs are doing.



.marko


--
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2015-01-22 12:37 GMT+01:00 Marko Tiikkaja :

> Hello,
>
> I just heard that there's going to be a fifth CF for 9.5 so I'm trying to
> gather all the patches I'd like to see in 9.5..
>
> On 8/23/13 10:36 AM, I wrote:
>
>> My opinion at this very moment is that we should leave the the DEFAULT
>> verbosity alone and add a new one (call it COMPACT or such) with the
>> suppressed context for non-ERRORs.
>>
>
> I wonder if a better option would be to add a GUC to control this from the
> server side.  plpgsql.suppress_simple_error_context or such, defaulting
> to false to maintain full backwards compatibility.  That could be set to
> true for development installations and for client programs which only care
> about having all information available, rather than readability or
> aesthetics.
>
> Or is that a stupid idea?  I just think hacking libpq for something like
> this is a huge overkill.
>

I don't think so only plpgsql  solution is satisfactory idea. There are
some mix plpgsql / plperl ... application - and it isn't possible to remove
error context from only one language.

Regards

Pavel


>
>
> .marko
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Marko Tiikkaja]

Hello,

I just heard that there's going to be a fifth CF for 9.5 so I'm trying 
to gather all the patches I'd like to see in 9.5..


On 8/23/13 10:36 AM, I wrote:

My opinion at this very moment is that we should leave the the DEFAULT
verbosity alone and add a new one (call it COMPACT or such) with the
suppressed context for non-ERRORs.


I wonder if a better option would be to add a GUC to control this from 
the server side.  plpgsql.suppress_simple_error_context or such, 
defaulting to false to maintain full backwards compatibility.  That 
could be set to true for development installations and for client 
programs which only care about having all information available, rather 
than readability or aesthetics.


Or is that a stupid idea?  I just think hacking libpq for something like 
this is a huge overkill.



.marko


--
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] PL/pgSQL, RAISE and error context

[Jeevan Chalke]

Hi Marko,

I have reviewed this patch.

1. Patch applies well.
2. make and make install is fine
3. make check is fine too.

But as Peter pointed out plperl regression tests are failing.

I just did grep on .sql files and found following files which has RAISE
statement into it. These files too need expected output changes. Please run
these testcases to get diffs.

./src/pl/plperl/sql/plperl_elog.sql
./src/pl/plpython/sql/plpython_error.sql
./src/pl/plpython/sql/plpython_setof.sql
./src/pl/plpython/sql/plpython_quote.sql
./contrib/sepgsql/sql/label.sql
./contrib/sepgsql/sql/ddl.sql

Code changes looks fine to me.

Thanks


-- 
Jeevan B Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Peter Eisentraut]

On Sun, 2013-09-15 at 14:28 +0200, Marko Tiikkaja wrote:
> On 15/09/2013 13:58, I wrote:
> > Hmm.  I just noticed there's something weird going on in the select_view
> > test.  I'm investigating this currently.
> 
> Seems that there's some magic going on and I overwrote the expected 
> results of the wrong file.  However, I can't figure out how one is 
> supposed to be getting the output of expected/select_views.out, nor do I 
> find this documented anywhere (I know xml has a similar thing so I tried 
> grepping around for XML, to no avail).
> 
> Here's an updated patch, but I think expected/select_views.out is still 
> broken.

You patch still fails the plperl regression tests.

I don't see a failure with select_views.  Your issue might be that you
updated expected/select_views_1.out but not expected/select_views.out.
This documentation might help:
http://www.postgresql.org/docs/devel/static/regress-variant.html



-- 
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] PL/pgSQL, RAISE and error context

[Marko Tiikkaja]

On 15/09/2013 13:50, I wrote:

On 15/09/2013 04:05, Peter Eisentraut wrote:

On Sat, 2013-09-14 at 04:58 +0200, Marko Tiikkaja wrote:

The attached patch (based on Pavel's patch) changes the default to be
slightly more verbose (the CONTEXT lines which were previously
omitted
will be visible), but adds a new PGVerbosity called COMPACT which
suppresses CONTEXT in non-error messages.  Now DEFAULT will be more
useful when debugging PL/PgSQL, and people who are annoyed by the new
behaviour can use the COMPACT mode.


Your patch fails the regression tests.


Attached is an updated patch with the regression test fixes.  No other
changes included.


Hmm.  I just noticed there's something weird going on in the select_view 
test.  I'm investigating this currently.



Regards,
Marko Tiikkaja


--
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] PL/pgSQL, RAISE and error context

[Peter Eisentraut]

On Sat, 2013-09-14 at 04:58 +0200, Marko Tiikkaja wrote:
> The attached patch (based on Pavel's patch) changes the default to be 
> slightly more verbose (the CONTEXT lines which were previously
> omitted 
> will be visible), but adds a new PGVerbosity called COMPACT which 
> suppresses CONTEXT in non-error messages.  Now DEFAULT will be more 
> useful when debugging PL/PgSQL, and people who are annoyed by the new 
> behaviour can use the COMPACT mode. 

Your patch fails the regression tests.



-- 
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2013/9/14 Marko Tiikkaja 

> On 23/08/2013 10:36, I wrote:
>
>> On 8/23/13 8:38 AM, Pavel Stehule wrote:
>>
>>> do you prepare patch ?
>>>
>>
>> I should have the time to produce one for the September commitfest, but
>> if you (or anyone else) want to work on this, I won't object.
>>
>> My opinion at this very moment is that we should leave the the DEFAULT
>> verbosity alone and add a new one (call it COMPACT or such) with the
>> suppressed context for non-ERRORs.
>>
>
> Well, turns out there isn't really any way to preserve complete backwards
> compatibility if we want to do this change.
>
> The attached patch (based on Pavel's patch) changes the default to be
> slightly more verbose (the CONTEXT lines which were previously omitted will
> be visible), but adds a new PGVerbosity called COMPACT which suppresses
> CONTEXT in non-error messages.  Now DEFAULT will be more useful when
> debugging PL/PgSQL, and people who are annoyed by the new behaviour can use
> the COMPACT mode.
>
> Any thoughts?
>
>
+1

Regards

Pavel


>
>
> Regards,
> Marko Tiikkaja
>
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Marko Tiikkaja]

On 23/08/2013 10:36, I wrote:

On 8/23/13 8:38 AM, Pavel Stehule wrote:

do you prepare patch ?


I should have the time to produce one for the September commitfest, but
if you (or anyone else) want to work on this, I won't object.

My opinion at this very moment is that we should leave the the DEFAULT
verbosity alone and add a new one (call it COMPACT or such) with the
suppressed context for non-ERRORs.


Well, turns out there isn't really any way to preserve complete 
backwards compatibility if we want to do this change.


The attached patch (based on Pavel's patch) changes the default to be 
slightly more verbose (the CONTEXT lines which were previously omitted 
will be visible), but adds a new PGVerbosity called COMPACT which 
suppresses CONTEXT in non-error messages.  Now DEFAULT will be more 
useful when debugging PL/PgSQL, and people who are annoyed by the new 
behaviour can use the COMPACT mode.


Any thoughts?



Regards,
Marko Tiikkaja

*** a/doc/src/sgml/libpq.sgml
--- b/doc/src/sgml/libpq.sgml
***
*** 5418,5423  int PQsetClientEncoding(PGconn 
*conn, const char *VERBOSE
!   mode includes all available fields.  Changing the verbosity does not
!   affect the messages available from already-existing
!   PGresult objects, only subsequently-created ones.
   
  
 
--- 5431,5442 
returned messages include severity, primary text, and position only;
this will normally fit on a single line.  The default mode produces
messages that include the above plus any detail, hint, or context
!   fields (these might span multiple lines).  The COMPACT mode is otherwise
!   the same as the default, except the context field will be omitted for
!   non-error messages.  The VERBOSE mode includes all
!   available fields.  Changing the verbosity does not affect the messages
!   available from already-existing PGresult objects, only
!   subsequently-created ones.
   
  
 
*** a/src/bin/psql/startup.c
--- b/src/bin/psql/startup.c
***
*** 796,801  verbosity_hook(const char *newval)
--- 796,803 
pset.verbosity = PQERRORS_DEFAULT;
else if (strcmp(newval, "terse") == 0)
pset.verbosity = PQERRORS_TERSE;
+   else if (strcmp(newval, "compact") == 0)
+   pset.verbosity = PQERRORS_COMPACT;
else if (strcmp(newval, "verbose") == 0)
pset.verbosity = PQERRORS_VERBOSE;
else
*** a/src/interfaces/libpq/fe-protocol3.c
--- b/src/interfaces/libpq/fe-protocol3.c
***
*** 915,920  pqGetErrorNotice3(PGconn *conn, bool isError)
--- 915,924 
if (val)
appendPQExpBuffer(&workBuf, libpq_gettext("QUERY:  
%s\n"), val);
val = PQresultErrorField(res, PG_DIAG_CONTEXT);
+   }
+   if (isError || (conn->verbosity != PQERRORS_TERSE &&
+   conn->verbosity != PQERRORS_COMPACT))
+   {
if (val)
appendPQExpBuffer(&workBuf, libpq_gettext("CONTEXT:  
%s\n"), val);
}
*** a/src/interfaces/libpq/libpq-fe.h
--- b/src/interfaces/libpq/libpq-fe.h
***
*** 106,111  typedef enum
--- 106,112 
  typedef enum
  {
PQERRORS_TERSE, /* single-line error messages */
+   PQERRORS_COMPACT,   /* single-line error messages 
on non-error messags */
PQERRORS_DEFAULT,   /* recommended style */
PQERRORS_VERBOSE/* all the facts, ma'am */
  } PGVerbosity;
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
***
*** 39,46 
  #include "utils/typcache.h"
  
  
- static const char *const raise_skip_msg = "RAISE";
- 
  typedef struct
  {
int nargs;  /* number of arguments 
*/
--- 39,44 
***
*** 867,876  plpgsql_exec_error_callback(void *arg)
  {
PLpgSQL_execstate *estate = (PLpgSQL_execstate *) arg;
  
-   /* if we are doing RAISE, don't report its location */
-   if (estate->err_text == raise_skip_msg)
-   return;
- 
if (estate->err_text != NULL)
{
/*
--- 865,870 
***
*** 3032,3038  exec_stmt_raise(PLpgSQL_execstate *estate, 
PLpgSQL_stmt_raise *stmt)
/*
 * 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,
--- 3026,3031 

-- 
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2013/8/23 Marko Tiikkaja 

> On 8/23/13 8:38 AM, Pavel Stehule wrote:
>
>  2013/8/22 Marko Tiikkaja 
>>
>>> I like the idea, but I think this should be a new verbosity level.  With
>>> this patch you would have to go full VERBOSE just to debug PL/pgSQL code
>>> with NOTICEs and DEBUGs in it, and that output then becomes harder to
>>> parse
>>> with the useless C-code information.
>>>
>>>
>> do you prepare patch ?
>>
>
> I should have the time to produce one for the September commitfest, but if
> you (or anyone else) want to work on this, I won't object.
>
> My opinion at this very moment is that we should leave the the DEFAULT
> verbosity alone and add a new one (call it COMPACT or such) with the
> suppressed context for non-ERRORs.
>


The name  is not important. What I would, for DEFAULT verbosity, to see a
context when RAISE EXCEPTION is used. It is a bug now, I think

Regards

Pavel


>
>
> Regards,
> Marko Tiikkaja
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Marko Tiikkaja]

On 8/23/13 8:38 AM, Pavel Stehule wrote:

2013/8/22 Marko Tiikkaja 

I like the idea, but I think this should be a new verbosity level.  With
this patch you would have to go full VERBOSE just to debug PL/pgSQL code
with NOTICEs and DEBUGs in it, and that output then becomes harder to parse
with the useless C-code information.



do you prepare patch ?


I should have the time to produce one for the September commitfest, but 
if you (or anyone else) want to work on this, I won't object.


My opinion at this very moment is that we should leave the the DEFAULT 
verbosity alone and add a new one (call it COMPACT or such) with the 
suppressed context for non-ERRORs.



Regards,
Marko Tiikkaja


--
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2013/8/22 Marko Tiikkaja 

> On 8/22/13 9:08 AM, Pavel Stehule wrote:
>
>> Probably we can introduce a new level of verbosity, but I am thinking so
>> this behave is reasonable. Everybody who use a VERBOSE level expect lot of
>> balast and it show expected info (context of error)
>>
>> Can be this design good enough for you?
>>
>
> I like the idea, but I think this should be a new verbosity level.  With
> this patch you would have to go full VERBOSE just to debug PL/pgSQL code
> with NOTICEs and DEBUGs in it, and that output then becomes harder to parse
> with the useless C-code information.
>

do you prepare patch ?

Pavel


>
>
> Regards,
> Marko Tiikkaja
>
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2013/8/22 Marko Tiikkaja 

> On 8/22/13 9:08 AM, Pavel Stehule wrote:
>
>> Probably we can introduce a new level of verbosity, but I am thinking so
>> this behave is reasonable. Everybody who use a VERBOSE level expect lot of
>> balast and it show expected info (context of error)
>>
>> Can be this design good enough for you?
>>
>
> I like the idea, but I think this should be a new verbosity level.  With
> this patch you would have to go full VERBOSE just to debug PL/pgSQL code
> with NOTICEs and DEBUGs in it, and that output then becomes harder to parse
> with the useless C-code information.
>
>

word "DEBUG" is not good - it is used for Postgres debugging as log level

Pavel


>
> Regards,
> Marko Tiikkaja
>
>

Re: [HACKERS] PL/pgSQL, RAISE and error context

[Marko Tiikkaja]

On 8/22/13 9:08 AM, Pavel Stehule wrote:

Probably we can introduce a new level of verbosity, but I am thinking so
this behave is reasonable. Everybody who use a VERBOSE level expect lot of
balast and it show expected info (context of error)

Can be this design good enough for you?


I like the idea, but I think this should be a new verbosity level.  With 
this patch you would have to go full VERBOSE just to debug PL/pgSQL code 
with NOTICEs and DEBUGs in it, and that output then becomes harder to 
parse with the useless C-code information.



Regards,
Marko Tiikkaja



--
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] PL/pgSQL, RAISE and error context

[Merlin Moncure]

On Thu, Aug 22, 2013 at 2:08 AM, Pavel Stehule  wrote:
> Probably we can introduce a new level of verbosity, but I am thinking so
> this behave is reasonable. Everybody who use a VERBOSE level expect lot of
> balast and it show expected info (context of error)
>
> Can be this design good enough for you?

yep :-).

merlin


-- 
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

Hello

I played with this topic little bit

If I understand, the main problem is in console (or pgAdmin) output.

create or replace function foo()
returns void as $$
begin
  for i in 1..5
  loop
raise notice '> *';
  end loop;
  raise exception '***';
end;
$$ language plpgsql;

postgres=# select foo();
NOTICE:  > *
NOTICE:  > *
NOTICE:  > *
NOTICE:  > *
NOTICE:  > *
ERROR:  ***
Time: 2.024 ms
postgres=# \set VER
VERBOSITY  VERSION
postgres=# \set VERBOSITY

postgres=# \set VERBOSITY

postgres=# \set VERBOSITY terse
postgres=# select foo();
NOTICE:  > *
NOTICE:  > *
NOTICE:  > *
NOTICE:  > *
NOTICE:  > *
ERROR:  ***
Time: 0.908 ms
postgres=# \set VERBOSITY verbose
postgres=# select foo();
NOTICE:  0: > *
LOCATION:  exec_stmt_raise, pl_exec.c:3051
NOTICE:  0: > *
LOCATION:  exec_stmt_raise, pl_exec.c:3051
NOTICE:  0: > *
LOCATION:  exec_stmt_raise, pl_exec.c:3051
NOTICE:  0: > *
LOCATION:  exec_stmt_raise, pl_exec.c:3051
NOTICE:  0: > *
LOCATION:  exec_stmt_raise, pl_exec.c:3051
ERROR:  P0001: ***
LOCATION:  exec_stmt_raise, pl_exec.c:3051

Time: 0.314 ms

I see a two little bit not nice issues:

a) in terse mode missing a CONTEXT for RAISED error
b) in verbose mode missing a CONTEXT for messages, for error too, and
useless LOCATION is showed.

LOCATION is absolutely useless for custom messages.

so I removed a context filtering

 postgres=# select foo();
NOTICE:  > *
CONTEXT:  PL/pgSQL function foo() line 5 at RAISE
NOTICE:  > *
CONTEXT:  PL/pgSQL function foo() line 5 at RAISE
NOTICE:  > *
CONTEXT:  PL/pgSQL function foo() line 5 at RAISE
NOTICE:  > *
CONTEXT:  PL/pgSQL function foo() line 5 at RAISE
NOTICE:  > *
CONTEXT:  PL/pgSQL function foo() line 5 at RAISE
ERROR:  ***
CONTEXT:  PL/pgSQL function foo() line 7 at RAISE
Time: 3.842 ms
postgres=# \set VERBOSITY verbose
postgres=# select foo();
NOTICE:  0: > *
CONTEXT:  PL/pgSQL function foo() line 5 at RAISE
LOCATION:  exec_stmt_raise, pl_exec.c:3046
NOTICE:  0: > *
CONTEXT:  PL/pgSQL function foo() line 5 at RAISE
LOCATION:  exec_stmt_raise, pl_exec.c:3046
NOTICE:  0: > *
CONTEXT:  PL/pgSQL function foo() line 5 at RAISE
LOCATION:  exec_stmt_raise, pl_exec.c:3046
NOTICE:  0: > *
CONTEXT:  PL/pgSQL function foo() line 5 at RAISE
LOCATION:  exec_stmt_raise, pl_exec.c:3046
NOTICE:  0: > *
CONTEXT:  PL/pgSQL function foo() line 5 at RAISE
LOCATION:  exec_stmt_raise, pl_exec.c:3046
ERROR:  P0001: ***
CONTEXT:  PL/pgSQL function foo() line 7 at RAISE
LOCATION:  exec_stmt_raise, pl_exec.c:3046
Time: 0.761 ms

We should not see a CONTEXT for DEFAULT verbosity and NOTICE level, after
little bit change I got a satisfied output


postgres=# select foo();
>>>NOTICE:  > *
>>>NOTICE:  > *
>>>NOTICE:  > *
>>>NOTICE:  > *
>>>NOTICE:  > *
ERROR:  ***
CONTEXT:  PL/pgSQL function foo() line 7 at RAISE
Time: 2.434 ms
postgres=# \set VERBOSITY verbose
postgres=# select foo();
>>>NOTICE:  0: > *
CONTEXT:  PL/pgSQL function foo() line 5 at RAISE
LOCATION:  exec_stmt_raise, pl_exec.c:3046
>>>NOTICE:  0: > *
CONTEXT:  PL/pgSQL function foo() line 5 at RAISE
LOCATION:  exec_stmt_raise, pl_exec.c:3046
>>>NOTICE:  0: > *
CONTEXT:  PL/pgSQL function foo() line 5 at RAISE
LOCATION:  exec_stmt_raise, pl_exec.c:3046
>>>NOTICE:  0: > *
CONTEXT:  PL/pgSQL function foo() line 5 at RAISE
LOCATION:  exec_stmt_raise, pl_exec.c:3046
>>>NOTICE:  0: > *
CONTEXT:  PL/pgSQL function foo() line 5 at RAISE
LOCATION:  exec_stmt_raise, pl_exec.c:3046
ERROR:  P0001: ***
CONTEXT:  PL/pgSQL function foo() line 7 at RAISE
LOCATION:  exec_stmt_raise, pl_exec.c:3046
Time: 0.594 ms

Probably we can introduce a new level of verbosity, but I am thinking so
this behave is reasonable. Everybody who use a VERBOSE level expect lot of
balast and it show expected info (context of error)

Can be this design good enough for you?

Regards

Pavel


plpgsql_raise_context.patch
Description: Binary data

-- 
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] PL/pgSQL, RAISE and error context

[Merlin Moncure]

On Wed, Aug 21, 2013 at 10:47 AM, Tom Lane  wrote:
> Merlin Moncure  writes:
>> On Wed, Aug 21, 2013 at 10:07 AM, Marko Tiikkaja  wrote:
>>> Why does  \set VERBOSITY 'terse'  not work for you?
>
>> Because it can't be controlled mid-function...that would suppress all
>> context of errors as well as messages and so it's useless.  Also psql
>> directives for this purpose is a hack anyways -- what if I'm using a
>> non-psql client?
>
>> what I really want is:
>> SET LOCAL log_console_verbosity = 'x'
>
> There was a protocol design decision a long time ago that verbosity
> ought to be controlled on the client side.  If we start suppressing
> fields server-side I think we're going to have problems.  In particular,
> I'm going to throw the "what if I'm not using psql" argument right back
> at you: what's the reason for thinking that a different client/application
> would have the identical desires about what fields to see?  It seems
> unlikely that a Java application, say, would want the server to be
> selective about what information it sends.

I didn't like that decision then and I don't like it now.  Why should
the protocol mandate that error context always be sent?  Why does this
have anything to do with the protocol at all? Just because we can't
imagine a case where a java application would not want context to be
sent in some or all contexts doesn't mean that operators should not
have control over it being emitted.  What harm could providing an
option possibly cause?

> I'm entirely prepared to believe that psql's VERBOSITY behavior could
> use more options, though.

How would that be structured... \set VERBOSITY 'NOTICE:terse'?

merlin


-- 
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] PL/pgSQL, RAISE and error context

[Tom Lane]

Merlin Moncure  writes:
> On Wed, Aug 21, 2013 at 10:07 AM, Marko Tiikkaja  wrote:
>> Why does  \set VERBOSITY 'terse'  not work for you?

> Because it can't be controlled mid-function...that would suppress all
> context of errors as well as messages and so it's useless.  Also psql
> directives for this purpose is a hack anyways -- what if I'm using a
> non-psql client?

> what I really want is:
> SET LOCAL log_console_verbosity = 'x'

There was a protocol design decision a long time ago that verbosity
ought to be controlled on the client side.  If we start suppressing
fields server-side I think we're going to have problems.  In particular,
I'm going to throw the "what if I'm not using psql" argument right back
at you: what's the reason for thinking that a different client/application
would have the identical desires about what fields to see?  It seems
unlikely that a Java application, say, would want the server to be
selective about what information it sends.

I'm entirely prepared to believe that psql's VERBOSITY behavior could
use more options, though.

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] PL/pgSQL, RAISE and error context

[Marko Tiikkaja]

On 2013-08-21 17:18, Merlin Moncure wrote:

On Wed, Aug 21, 2013 at 10:07 AM, Marko Tiikkaja  wrote:

Why does  \set VERBOSITY 'terse'  not work for you?


Because it can't be controlled mid-function...that would suppress all
context of errors as well as messages and so it's useless.


Fair enough.


what I really want is:
SET LOCAL log_console_verbosity = 'x'


log_min_messages vs. client_min_messages, so client_error_verbosity 
sounds more appropriate IMO.



Regards,
Marko Tiikkaja


--
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] PL/pgSQL, RAISE and error context

[Pavel Stehule]

2013/8/21 Merlin Moncure 

> On Wed, Aug 21, 2013 at 10:07 AM, Marko Tiikkaja  wrote:
> > On 8/21/13 5:05 PM, Merlin Moncure wrote:
> >>
> >> On Wed, Aug 21, 2013 at 9:22 AM, Tom Lane  wrote:
> >>>
> >>> Marko Tiikkaja  writes:
> 
>  By default, PL/pgSQL does not print the error context of a RAISE
>  statement, for example:
> >>>
> >>>
> >>> It used to do so, in the beginning when we first added
> context-printing.
> >>> There were complaints that the result was too verbose; for instance if
> >>> you
> >>> had a RAISE NOTICE inside a loop for progress-monitoring purposes,
> you'd
> >>> get two lines for every one you wanted.  I think if we undid this we'd
> >>> get the same complaints again.  I agree that in complicated nests of
> >>> functions the location info is more interesting than it is in trivial
> >>> cases, but that doesn't mean you're not going to hear such complaints
> >>> from
> >>> people with trivial functions.
> >>
> >>
> >> It *is* (apologies for the hijack) too verbose but whatever context
> >> suppressing we added doesn't work in pretty much any interesting case.
> >>   What is basically needed is for the console to honor
> >> log_error_verbosity (which I would prefer) or a separate GUC in manage
> >> the console logging verbosity:
> >
> >
> > Why does  \set VERBOSITY 'terse'  not work for you?
>
> Because it can't be controlled mid-function...that would suppress all
> context of errors as well as messages and so it's useless.  Also psql
> directives for this purpose is a hack anyways -- what if I'm using a
> non-psql client?
>
> what I really want is:
> SET LOCAL log_console_verbosity = 'x'
>
>
it is not bad idea

Pavel


>  merlin
>
>
> --
> 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] PL/pgSQL, RAISE and error context

[Merlin Moncure]

On Wed, Aug 21, 2013 at 10:07 AM, Marko Tiikkaja  wrote:
> On 8/21/13 5:05 PM, Merlin Moncure wrote:
>>
>> On Wed, Aug 21, 2013 at 9:22 AM, Tom Lane  wrote:
>>>
>>> Marko Tiikkaja  writes:

 By default, PL/pgSQL does not print the error context of a RAISE
 statement, for example:
>>>
>>>
>>> It used to do so, in the beginning when we first added context-printing.
>>> There were complaints that the result was too verbose; for instance if
>>> you
>>> had a RAISE NOTICE inside a loop for progress-monitoring purposes, you'd
>>> get two lines for every one you wanted.  I think if we undid this we'd
>>> get the same complaints again.  I agree that in complicated nests of
>>> functions the location info is more interesting than it is in trivial
>>> cases, but that doesn't mean you're not going to hear such complaints
>>> from
>>> people with trivial functions.
>>
>>
>> It *is* (apologies for the hijack) too verbose but whatever context
>> suppressing we added doesn't work in pretty much any interesting case.
>>   What is basically needed is for the console to honor
>> log_error_verbosity (which I would prefer) or a separate GUC in manage
>> the console logging verbosity:
>
>
> Why does  \set VERBOSITY 'terse'  not work for you?

Because it can't be controlled mid-function...that would suppress all
context of errors as well as messages and so it's useless.  Also psql
directives for this purpose is a hack anyways -- what if I'm using a
non-psql client?

what I really want is:
SET LOCAL log_console_verbosity = 'x'

merlin


-- 
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] PL/pgSQL, RAISE and error context

[Marko Tiikkaja]

On 8/21/13 5:05 PM, Merlin Moncure wrote:

On Wed, Aug 21, 2013 at 9:22 AM, Tom Lane  wrote:

Marko Tiikkaja  writes:

By default, PL/pgSQL does not print the error context of a RAISE
statement, for example:


It used to do so, in the beginning when we first added context-printing.
There were complaints that the result was too verbose; for instance if you
had a RAISE NOTICE inside a loop for progress-monitoring purposes, you'd
get two lines for every one you wanted.  I think if we undid this we'd
get the same complaints again.  I agree that in complicated nests of
functions the location info is more interesting than it is in trivial
cases, but that doesn't mean you're not going to hear such complaints from
people with trivial functions.


It *is* (apologies for the hijack) too verbose but whatever context
suppressing we added doesn't work in pretty much any interesting case.
  What is basically needed is for the console to honor
log_error_verbosity (which I would prefer) or a separate GUC in manage
the console logging verbosity:


Why does  \set VERBOSITY 'terse'  not work for you?



Regards,
Marko Tiikkaja



--
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] PL/pgSQL, RAISE and error context

[Merlin Moncure]

On Wed, Aug 21, 2013 at 9:22 AM, Tom Lane  wrote:
> Marko Tiikkaja  writes:
>> By default, PL/pgSQL does not print the error context of a RAISE
>> statement, for example:
>
> It used to do so, in the beginning when we first added context-printing.
> There were complaints that the result was too verbose; for instance if you
> had a RAISE NOTICE inside a loop for progress-monitoring purposes, you'd
> get two lines for every one you wanted.  I think if we undid this we'd
> get the same complaints again.  I agree that in complicated nests of
> functions the location info is more interesting than it is in trivial
> cases, but that doesn't mean you're not going to hear such complaints from
> people with trivial functions.

It *is* (apologies for the hijack) too verbose but whatever context
suppressing we added doesn't work in pretty much any interesting case.
 What is basically needed is for the console to honor
log_error_verbosity (which I would prefer) or a separate GUC in manage
the console logging verbosity:

set log_error_verbosity = 'terse';
SET

CREATE OR REPLACE FUNCTION Notice(_msg TEXT) RETURNS VOID AS
$$
BEGIN
  RAISE NOTICE '[%] %', clock_timestamp()::timestamp(0)::text, _msg;
END;
$$ LANGUAGE PLPGSQL;

CREATE OR REPLACE FUNCTION foo() RETURNS VOID AS
$$
BEGIN
  PERFORM Notice('test');
END;
$$ LANGUAGE PLPGSQL;

-- context will print
postgres=# select foo();
NOTICE:  [2013-08-21 09:52:08] test
CONTEXT:  SQL statement "SELECT Notice('test')"
PL/pgSQL function foo() line 4 at PERFORM

CREATE OR REPLACE FUNCTION bar() RETURNS VOID AS
$$
  SELECT Notice('test');
$$ LANGUAGE SQL;

-- context will not print
postgres=# select bar();
NOTICE:  [2013-08-21 09:54:55] test

-- context will  print
CREATE OR REPLACE FUNCTION baz() RETURNS VOID AS
$$
  select 0;
  SELECT Notice('test');
$$ LANGUAGE SQL;

postgres=# select baz();
NOTICE:  [2013-08-21 09:55:26] test
CONTEXT:  SQL function "baz" statement 2

merlin


-- 
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] PL/pgSQL, RAISE and error context

[Marko Tiikkaja]

On 8/21/13 4:22 PM, Tom Lane wrote:

Marko Tiikkaja  writes:

By default, PL/pgSQL does not print the error context of a RAISE
statement, for example:


It used to do so, in the beginning when we first added context-printing.
There were complaints that the result was too verbose; for instance if you
had a RAISE NOTICE inside a loop for progress-monitoring purposes, you'd
get two lines for every one you wanted.  I think if we undid this we'd
get the same complaints again.  I agree that in complicated nests of
functions the location info is more interesting than it is in trivial
cases, but that doesn't mean you're not going to hear such complaints from
people with trivial functions.


They have an option: they can reduce verbosity in their client.  I 
currently don't have any real options.



Regards,
Marko Tiikkaja


--
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] PL/pgSQL, RAISE and error context

[Tom Lane]

Marko Tiikkaja  writes:
> By default, PL/pgSQL does not print the error context of a RAISE 
> statement, for example:

It used to do so, in the beginning when we first added context-printing.
There were complaints that the result was too verbose; for instance if you
had a RAISE NOTICE inside a loop for progress-monitoring purposes, you'd
get two lines for every one you wanted.  I think if we undid this we'd
get the same complaints again.  I agree that in complicated nests of
functions the location info is more interesting than it is in trivial
cases, but that doesn't mean you're not going to hear such complaints from
people with trivial functions.

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