Re: options for RAISE statement

Hello

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

Thank You
Pavel Stehule

2008/5/5 Tom Lane <tgl@sss.pgh.pa.us>:
> "Pavel Stehule" <pavel.stehule@gmail.com> 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
>