Re: review: CHECK FUNCTION statement

On 01/01/2012 01:01 PM, Pavel Stehule wrote:
> Hello all
>
> here is new version of CHECK FUNCTION patch

Hi,

I took a shot at reviewing this. The attached version is made against
yesterdays HEAD (which means it applies cleanly) with some updates to
documentation.

> I changed implementation of interface:
>
> * checked functions returns table instead raising exceptions - it
> necessary for describing more issues inside one function - and it
> allow to use better structured data then ExceptionDat

The new interface makes sense to me the way it is, should be usable by
other languages too and for external tools it should enable sufficient
filtering options for what to care about and what not to care about.

Anyway on to full review:

Submission:
Patch has enough documentation (mostly written by Albe Laurenz with some
adjustments from me). It has quite nice set of regression tests which it
passes on both my machines.

Usability:
We certainly want (IMHO it's something we should have had long time ago)
this feature and patch implements it in a way that seems to be useful.
It has pg_dump support.

Feature test:
Works as advertised, passes all regression tests, I tested many real
world functions written by various people and was unable to crash it or
make it misbehave. I can imagine it not working properly with some
EXECUTE statements but I don't believe that is avoidable due to the
nature of PL/pgSQL.

Coding review:
I have no complaints about the code itself, neither did my compiler.
There is no interaction with system so it should not cause any
portability issues.

From my point of view this seems to be ready for committer and barring
any objections I will mark it as such.

Regards
Petr Jelinek (PJMODOS)