Re: options for RAISE statement

From: "Pavel Stehule" <pavel(dot)stehule(at)gmail(dot)com>
To: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: options for RAISE statement
Date: 2008-05-10 05:27:09
Message-ID: 162867790805092227k77f31771xbe02747c30039bb3@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches

Hello

I am sending enhanced version of original patch.

2008/5/5 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> "Pavel Stehule" <pavel(dot)stehule(at)gmail(dot)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?

There are new attribut CONDITION - all defined condition are possible
without duplicit names and category conditions.

example:
RAISE NOTICE 'custom unique violation' USING (CONDITION = 'unique_violation');

>
> * If we are going to let people throw random SQLSTATEs, there had better
> be a way to name those same SQLSTATEs in EXCEPTION.
>
we can trap EXCEPTION defined via SQLSTATE now:

EXCEPTION
WHEN SQLSTATE 22001 THEN ...

> * 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.

removed

>
> * 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.
>

changed
> * // comments are against our coding conventions.
>
> regards, tom lane
>

removed

Regards
Pavel Stehule

Attachment Content-Type Size
raise-options.diff text/x-patch 24.4 KB

In response to

Responses

Browse pgsql-patches by date

  From Date Subject
Next Message Andreas 'ads' Scherbaum 2008-05-10 07:36:26 Re: create or replace language
Previous Message Bruce Momjian 2008-05-10 01:32:17 Re: Database owner installable modules patch