(2011/06/02 17:39), Pavel Stehule wrote:
> This patch enhances a GET DIAGNOSTICS statement functionality. It adds
> a possibility of access to exception's data. These data are stored on
> stack when exception's handler is activated - and these data are
> access-able everywhere inside handler. It has a different behave (the
> content is immutable inside handler) and therefore it has modified
> syntax (use keyword STACKED). This implementation is in conformance
> with ANSI SQL and SQL/PSM  - implemented two standard fields -
> RETURNED_SQLSTATE and MESSAGE_TEXT and three PostgreSQL specific
> fields - PG_EXCEPTION_DETAIL, PG_EXCEPTION_HINT and
> PG_EXCEPTION_CONTEXT.
> 
> The GET STACKED DIAGNOSTICS statement is allowed only inside
> exception's handler. When it is used outside handler, then diagnostics
> exception 0Z002 is raised.
> 
> This patch has no impact on performance. It is just interface to
> existing stacked 'edata' structure. This patch doesn't change a
> current behave of GET DIAGNOSTICS statement.
> 
> CREATE OR REPLACE FUNCTION public.stacked_diagnostics_test02()
>   RETURNS void
>   LANGUAGE plpgsql
> AS $function$
> declare _detail text; _hint text; _message text;
> begin
>    perform ...
> exception when others then
>    get stacked diagnostics
>          _message = message_text,
>          _detail = pg_exception_detail,
>          _hint = pg_exception_hint;
>    raise notice 'message: %, detail: %, hint: %', _message, _detail, _hint;
> end;
> $function$
> 
> All regress tests was passed.

Hi Pavel,

I've reviewed your patch according to the page "Reviewing a patch".
During the review, I referred to Working-Draft of SQL 2003 to confirm
the SQL specs.

Submission review
=================
* The patch is in context diff format.
* The patch couldn't be applied cleanly to the current head.  But it
requires only one hunk to be offset, and it could be fixed easily.
I noticed that new variables needs_xxx, which were added to struct
PLpgSQL_condition, are not used at all.  They should be removed, or
something might be overlooked.
* The patch includes reasonable regression tests.  The patch also
includes hunks for pl/pgsql document which describes new
feature.  But it would need some corrections:
  - folding too-long lines
  - fixing some grammatical errors (maybe)
  - clarify difference between CURRENT and STACKED
I think that adding new section for GET STACKED DIAGNOSTICS would help
to clarify the difference, because the keyword STACKED can be used only
in exception clause, and available information is different from the one
available for GET CURRENT DIAGNOSTICS.  Please find attached a patch
which includes a proposal for document though it still needs review by
English speaker.

Usability review
================
* The patch extends GET DIAGNOSTICS syntax to accept new keywords
CURRENT and STACKED, which are described in the SQL/PSM standard.  This
feature allows us to retrieve exception information in EXCEPTION clause.
Naming of PG-specific fields might be debatable.
* I think it's useful to get detailed information inside EXCEPTION clause.
* We don't have this feature yet.
* This patch follows SQL spec of GET DIAGNOSTICS, and extends about
PG-specific variables.
* pg_dump support is not required for this feature.
* AFAICS, this patch doesn't have any danger, such as breakage of
backward compatibility.

Feature test
============
* The new feature introduced by the patch works well.
I tested about:
  - CURRENT doesn't affect existing feature
  - STACKED couldn't be used outside EXCEPTION clause
  - Values could be retrieved via RETURNED_SQLSTATE, MESSAGE_TEXT,
    PG_EXCEPTION_DETAIL, PG_EXCEPTION_HINT and PG_EXCEPTION_CONTEXT
  - Invalid item names properly cause error.
* I'm not so familiar to pl/pgsql, but ISTM that enough cases are
considered about newly added diagnostics items.
* I didn't see any crash during my tests.

In conclusion, this patch still needs some effort to be "Ready for
Committer", so I'll push it back to "Waiting on Author".

Regards,
-- 
Shigeru Hanada
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index 3d07b6e..7df69a7 100644
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
*************** EXECUTE format('UPDATE tbl SET %I = $1 W
*** 1387,1393 ****
       command, which has the form:
  
  <synopsis>
! GET <optional> CURRENT | STACKED </optional> DIAGNOSTICS 
<replaceable>variable</replaceable> = <replaceable>item</replaceable> 
<optional> , ... </optional>;
  </synopsis>
  
       This command allows retrieval of system status indicators.  Each
--- 1387,1393 ----
       command, which has the form:
  
  <synopsis>
! GET <optional> CURRENT </optional> DIAGNOSTICS 
<replaceable>variable</replaceable> = <replaceable>item</replaceable> 
<optional> , ... </optional>;
  </synopsis>
  
       This command allows retrieval of system status indicators.  Each
*************** GET DIAGNOSTICS integer_var = ROW_COUNT;
*** 1486,1506 ****
       affect only the current function.
      </para>
  
      <para>
!       Inside a exception handler is possible to use a stacked diagnostics 
statement. It 
!       allows a access to exception's data: the 
<varname>RETURNED_SQLSTATE</varname> contains
!       a SQLSTATE of handled exception. <varname>MESSAGE_TEXT</varname> 
contains a message text,
!       <varname>PG_EXCEPTION_DETAIL</varname> has a text that is shown as 
exception detail,
!       <varname>PG_EXCEPTION_HINT</varname> has a hint related to catched 
exception.
!       <varname>PG_EXCEPTION_CONTEXT</varname> contains a lines that describes 
call stack. These
!       variables holds a text value. When some field of exception are not 
filled, then related 
!       variable contains a empty string,
      </para>
  
      <para>
!      An example:
  <programlisting>
  BEGIN
    ...
  EXCEPTION WHEN OTHERS THEN
    GET STACKED DIAGNOSTICS text_var1 = MESSAGE_TEXT,
--- 1486,1523 ----
       affect only the current function.
      </para>
  
+    </sect2>
+ 
+    <sect2 id="plpgsql-exception-diagnostics">
+     <title>Obtaining the Exception Status</title>
+ 
      <para>
!      Inside an exception handler, it's possible to retrieve detailed
!      information about the exception which is currently handled, with using a
!      <command>GET STACKED DIAGNOSTICS</command> command, which has the form:
! 
! <synopsis>
! GET STACKED DIAGNOSTICS <replaceable>variable</replaceable> = 
<replaceable>item</replaceable> <optional> , ... </optional>;
! </synopsis>
      </para>
  
      <para>
!      It allows you to access to exception's data: the
!      <varname>RETURNED_SQLSTATE</varname> contains a SQLSTATE of handled
!      exception. <varname>MESSAGE_TEXT</varname> contains a message text,
!      <varname>PG_EXCEPTION_DETAIL</varname> has a text that is shown as
!      exception detail, <varname>PG_EXCEPTION_HINT</varname> has a hint
!      related to catched exception.  <varname>PG_EXCEPTION_CONTEXT</varname>
!      contains a lines that describes call stack. These variables holds a text
!      value. When some of exception fields are not filled, then such variable
!      contains an empty string.  An example is:
  <programlisting>
+ DECLARE
+   text_var1 text;
+   text_var2 text;
+   text_var3 text;
  BEGIN
+   -- some processing which might cause an exception
    ...
  EXCEPTION WHEN OTHERS THEN
    GET STACKED DIAGNOSTICS text_var1 = MESSAGE_TEXT,
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to