Re: proposal: plpgsql - Assert statement

2015-03-25 0:17 GMT+01:00 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:

> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> > updated version with Jim Nasby's doc and rebase against last changes in
> > plpgsql.
>
> I started looking at this patch. ISTM there are some pretty questionable
> design decisions in it:
>
> 1. Why create a core GUC to control a behavior that's plpgsql-only?
> I think it'd make more sense for this to be a plgsql custom GUC
> (ie, "plpgsql.enable_asserts" or some such name).
>

This type of assertations can be implemented in any PL language - so I
prefer global setting. But I have not strong option in this case - this is
question about granularity - and more ways are valid.

>
> 2. I find the use of errdetail to report whether the assertion condition
> evaluated to FALSE or to NULL to be pretty useless. (BTW, is considering
> NULL to be a failure the right thing? SQL CHECK conditions consider NULL
> to be allowed ...)

This is a question - I am happy with SQL CHECK for data, but I am not sure
if same behave is safe for plpgsql (procedural) assert. More stricter
behave is safer - and some bugs in procedures are based on unhandled NULLs
in variables. So in this topic I prefer implemented behave. It is some like:

IF expression THEN
-- I am able do all
...
ELSE
RAISE EXCEPTION 'some is wrong';
END IF;

> I also don't care at all for reporting the internal
> text of the assertion expression in the errdetail: that will expose
> implementation details (ie, exactly what does plpgsql convert the user
> expression to, does it remove comments, etc) which we will then be
> constrained from changing for fear of breaking client code that expects a
> particular spelling of the condition. I think we should just drop that
> whole business. The user can report the condition in her message, if she
> feels the need to.
>
>
+1

> 3. If we drop the errdetail as per #2, then reporting the optional
> user-supplied string as a HINT would be just plain bizarre; not that it
> wasn't bizarre already, because there's no good reason to suppose that
> whatever the programmer has to say about the assertion is merely a hint.
> I also find the behavior for string-evaluates-to-NULL bizarre; it'd be
> saner just to leave out the message field, same as if the argument weren't
> there. I would suggest that we adopt one of these two definitions for the
> optional string:
>
> 3a. If string is present and not null, use it as the primary message text
> (otherwise use "assertion failed").
>
> 3b. If string is present and not null, use it as errdetail, with the
> primary message text always being "assertion failed".
>
> I mildly prefer #3a, but could be talked into #3b.
>

I prefer #3b - there is more informations.

Regards

Pavel

>
> Comments?
>
> regards, tom lane
>