Re: proposal: plpgsql - Assert statement

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

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

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.

Comments?

regards, tom lane