I thing, all your comments are not problem. I'll send new version this week.

Thank You
Pavel Stehule

2008/5/5 Tom Lane <[EMAIL PROTECTED]>:
> "Pavel Stehule" <[EMAIL PROTECTED]> writes:
>> this patch adds possibility to set additional options (SQLSTATE,
>> DETAIL, DETAIL_LOG and HINT) for RAISE statement,
> I looked this over briefly.  A couple of comments:
> * Raising errors via hard-coded SQLSTATEs seems pretty unfriendly,
> at least for cases where we are reporting built-in errors.  Wouldn't
> it be better to be able to raise errors using the same SQLSTATE names
> that are recognized in EXCEPTION clauses?
> * If we are going to let people throw random SQLSTATEs, there had better
> be a way to name those same SQLSTATEs in EXCEPTION.
> * I don't really like exposing DETAIL_LOG in this.  That was a spur of
> the moment addition and we might take it out again; I think it's way
> premature to set it in stone by exposing it as a plpgsql feature.
> * Please avoid using errstart() directly.  This is unwarranted intimacy
> with elog.h's implementation and I also think it will have unpleasant
> behavior if an error occurs while evaluating the RAISE arguments.
> (In fact, I think a user could easily force a backend PANIC that way.)
> The approved way to deal with ereport options that might not be there
> is like this:
>        ereport(ERROR,
>                ( ...,
>                 have_sqlstate ? errcode(...) : 0,
>                  ...
> That is, you should evaluate all the options into local variables
> and then do one normal ereport call.
> * // comments are against our coding conventions.
>                        regards, tom lane

Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:

Reply via email to