Thread: proposal: enhanced get diagnostics statement

Hello

This proposal is related to exception processing. Inside exception
handler we can get some basic info about exception - message text and
message code. There are other fields - but these fields are no
available now in PL/pgSQL. The cheap access to fields inside ErrorData
structure can be implemented inside GET DIAGNOSTICS statements - this
statement is created for this purpose. I propose a new thee
identifiers, that can be used there: ERROR_DETAIL, ERROR_HINT and
ERROR_CONTEXT. Using is simple:

CREATE OR REPLACE FUNCTION foo()
RETURNS void AS $$
DECLARE
  _detail text;
  _hint text;
  _context text;
BEGIN
  RAISE EXCEPTION 'some message'
    USING DETAIL = 'some message specific description',
          HINT = 'some hint related to messgae';

EXCEPTION WHEN OTHERS THEN
  GET DIAGNOSTICS _detail = ERROR_DETAIL,
                  _hint = ERROR_HINT,
                  _context = ERROR_CONTEXT;

  RAISE WARNING 'caught message: %', SQLERRM
    USING DETAIL = e'\ncaught detail: ' || _detail ||
                   e'\ncaught hint: ' || _hint ||
                   e'\ncaught context: ' || _context;

END;
$$ LANGUAGE plpgsql;
SELECT foo();


A implementation of ERROR_DETAIL and ERROR_HINT is simple and without
possible performance issues. It has zero impact on performance.

A implementation of ERROR_CONTEXT is not without impact on
performance, because context should be collected when exception is
caught. One solution is removing a ERROR_CONTEXT from proposal. Second
solution can be a design of enhanced syntax for exception trap like
(it means - collect CONTEXT when exception is handled)

BEGIN
  EXCEPTION (ERROR_CONTEXT=true) WHEN OTHERS THEN
    ...
END

Getting a context can be a problem - but it is very important
information, that can significantly help with exception's explanation.

ideas, notes?

Regards

Pavel Stehule

Excerpts from Pavel Stehule's message of sáb may 21 16:05:01 -0400 2011:

> A implementation of ERROR_CONTEXT is not without impact on
> performance, because context should be collected when exception is
> caught. One solution is removing a ERROR_CONTEXT from proposal. Second
> solution can be a design of enhanced syntax for exception trap like
> (it means - collect CONTEXT when exception is handled)

I don't understand why we should worry about this.  I mean, if you don't
catch the error, you have to form errcontext anyway.  Why is it a
problem to generate it when the exception is caught?

-- 
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

2011/5/21 Alvaro Herrera <alvherre@commandprompt.com>:
> Excerpts from Pavel Stehule's message of sáb may 21 16:05:01 -0400 2011:
>
>> A implementation of ERROR_CONTEXT is not without impact on
>> performance, because context should be collected when exception is
>> caught. One solution is removing a ERROR_CONTEXT from proposal. Second
>> solution can be a design of enhanced syntax for exception trap like
>> (it means - collect CONTEXT when exception is handled)
>
> I don't understand why we should worry about this.  I mean, if you don't
> catch the error, you have to form errcontext anyway.  Why is it a
> problem to generate it when the exception is caught?

Generating context means a calling a few functions with some string
operations - because in this moment is limited functionality, there
isn't too much operations - but it can be changed - PL/PSM dumps all
local variables, ...

somebody uses a exception trapping like mechanism for ignoring errors

FOR r IN SELECT ..
LOOP BEGIN   INSERT INTO ... EXCEPTION WHEN OTHERS THEN   /* do nothing */ END;
END LOOP;

or some body can do

BEGIN ...
EXCEPTION WHEN OTHERS THEN RAISE WARNING ' .....'; RAISE;
END;

In last case the context can wanted - so it cannot be hard problem.
But first case is problem and we has not different way how to do it.

Maybe we can use a simple optimization

when function doesn't contain a GET DIAGNOSTICS statement with
ERROR_CONTEXT field, then we can not collect a context. Only when
function has GET DIAGNOSTICS with ERROR_CONTEXT we will take context
info. This strategy ensure, so there will not be negative performance
effect on current applications.

Pavel

>
> --
> Álvaro Herrera <alvherre@commandprompt.com>
> The PostgreSQL Company - Command Prompt, Inc.
> PostgreSQL Replication, Consulting, Custom Development, 24x7 support
>

Alvaro Herrera <alvherre@commandprompt.com> writes:
> Excerpts from Pavel Stehule's message of sáb may 21 16:05:01 -0400 2011:
>> A implementation of ERROR_CONTEXT is not without impact on
>> performance, because context should be collected when exception is
>> caught. One solution is removing a ERROR_CONTEXT from proposal. Second
>> solution can be a design of enhanced syntax for exception trap like
>> (it means - collect CONTEXT when exception is handled)

> I don't understand why we should worry about this.  I mean, if you don't
> catch the error, you have to form errcontext anyway.  Why is it a
> problem to generate it when the exception is caught?

The argument is nonsense anyway, because it's based on an incorrect
implementation.  exec_stmt_block has no business calling the error
context stack --- that was already done in errfinish.  Worse, by the time
you get control back from the longjmp, you probably have popped off some
of the stack variables that those links would need to use.  If that code
didn't dump core in testing, it's likely only because it wasn't tested
very much.


A bigger issue is that it seems like this is an abuse of GET
DIAGNOSTICS.  The existing options of that statement report on the
results of the immediately preceding command.  If we add options that
report on the last error, then we will have a situation where some of
the options have different "lifespan" than others.  That seems pretty
klugy and confusing to me.

I'm not real sure what to do instead.  If we were actually trying to
follow precedent here, what we'd be looking to do is add more
auto-initialized variables like SQLERRM inside recovery blocks.
That would again have the issue of wasting cycles in code that never
actually looked at the variables.  (But I wonder if we couldn't tweak
plpgsql so that we could determine at compile time which variables have
references and which don't, so that we'd not bother filling in variables
that weren't used.  Or somehow arrange to only do the work when the
variable's value is accessed.)
        regards, tom lane

2011/5/22 Tom Lane <tgl@sss.pgh.pa.us>:
> Alvaro Herrera <alvherre@commandprompt.com> writes:
>> Excerpts from Pavel Stehule's message of sáb may 21 16:05:01 -0400 2011:
>>> A implementation of ERROR_CONTEXT is not without impact on
>>> performance, because context should be collected when exception is
>>> caught. One solution is removing a ERROR_CONTEXT from proposal. Second
>>> solution can be a design of enhanced syntax for exception trap like
>>> (it means - collect CONTEXT when exception is handled)
>
>> I don't understand why we should worry about this.  I mean, if you don't
>> catch the error, you have to form errcontext anyway.  Why is it a
>> problem to generate it when the exception is caught?
>
> The argument is nonsense anyway, because it's based on an incorrect
> implementation.  exec_stmt_block has no business calling the error
> context stack --- that was already done in errfinish.  Worse, by the time
> you get control back from the longjmp, you probably have popped off some
> of the stack variables that those links would need to use.  If that code
> didn't dump core in testing, it's likely only because it wasn't tested
> very much.
>

It works with copy of ErrorData, that is available in estate - when
estate->cur_err is NOT NULL. I am not sure about "context" - the my
implementation is strange, but access to detail and hint is without
any known problems. Please, can you specify a situation, when access
to ErrorData fields can be broken?

>
> A bigger issue is that it seems like this is an abuse of GET
> DIAGNOSTICS.  The existing options of that statement report on the
> results of the immediately preceding command.  If we add options that
> report on the last error, then we will have a situation where some of
> the options have different "lifespan" than others.  That seems pretty
> klugy and confusing to me.
>

It doesn't mean a "last error", it means a error in current handler
block - because life of cur_err is limited per block. But you has
true. ANSI SQL defines "stacked diagnostics" statement for access to
error info about outer exceptions. The expected information is
available only when GET STATEMENT is first statement of exception
handler. If isn't first statement, you should to use a stacked GET
DIAGNOSTICS statement.

> I'm not real sure what to do instead.  If we were actually trying to
> follow precedent here, what we'd be looking to do is add more
> auto-initialized variables like SQLERRM inside recovery blocks.
> That would again have the issue of wasting cycles in code that never
> actually looked at the variables.  (But I wonder if we couldn't tweak
> plpgsql so that we could determine at compile time which variables have
> references and which don't, so that we'd not bother filling in variables
> that weren't used.  Or somehow arrange to only do the work when the
> variable's value is accessed.)

Any new magic values are not good - hard to enhance it. The correct
way is implementation GET DIAGNOSTICS statement based on ANSI/SQL. But
should be implemented different, it has to return exception info, only
when GD is first statement of handler block. Is it less confusing for
you?

Some optimization for SQLERRM can be done still - I am doing some
similar in PL/PSM and it is possible.

Regards

Pavel Stehule


>
>                        regards, tom lane
>