2015-03-25 0:17 GMT+01:00 Tom Lane <t...@sss.pgh.pa.us>: > Pavel Stehule <pavel.steh...@gmail.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 >