Re: patch: enhanced get diagnostics statement 2 - Mailing list pgsql-hackers
From | Pavel Stehule |
---|---|
Subject | Re: patch: enhanced get diagnostics statement 2 |
Date | |
Msg-id | CAFj8pRAsTgBzHSwLGgSzQOUi1kbWsbrswRSs_HYWSNDVAfv0MQ@mail.gmail.com Whole thread Raw |
In response to | Re: patch: enhanced get diagnostics statement 2 (Shigeru Hanada <shigeru.hanada@gmail.com>) |
Responses |
Re: patch: enhanced get diagnostics statement 2
|
List | pgsql-hackers |
Hello thank you very much for review. I cleaned patch and merged your documentation patch I hope, this is all - a language correction should do some native speaker Regards Pavel Stehule 2011/7/6 Shigeru Hanada <shigeru.hanada@gmail.com>: > (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 >
Attachment
pgsql-hackers by date: