Re: review: CHECK FUNCTION statement

"Albe Laurenz" <laurenz(dot)albe(at)wien(dot)gv(dot)at> writes:
> Do I understand right that the reason why the check function is
> different from the validator function is that it would be more difficult
> to add the checks to the validator function?

> Is that a good enough argument? From a user's perspective it is
> difficult to see why some checks are performed at function creation
> time, while others have to be explicitly checked with CHECK FUNCTION.
> I think it would be much more intuitive if CHECK FUNCTION does
> the same as function validation with check_function_bodies on.

I think the important point here is that we need to support more than
one level of validation, and that the higher levels can't really be
applied by default in CREATE FUNCTION because they may fail on perfectly
valid code.

The real reason why we need a separate check function is that the API
for validators doesn't include any parameter about check level.

It's conceivable that instead of adding a check-function entry point,
we could generalizee check_function_bodies into a more-than-2-level
setting, and expect validators to pay attention to the GUC's value
when deciding how aggressively to validate. However, it's far from
clear to me that that's a more usable definition than having a separate
CHECK FUNCTION command. An example of where a separate CHECK command
could come in handy is: you just did some ALTER TABLE commands, and now
you would like to check if your functions all still work with the
modified table schemas.

> [ snip examples of some additional checks that could be made ]
> It is probably very hard to check everything possible, but the
> usefulness of CHECK FUNCTION is proportional to the number of
> cases covered.

I don't think that the initial patch needs to check everything that
could conceivably be checked. We can always add more checking later.
The initial patch obviously has to create the infrastructure for
optional checking, and the specific check that Pavel wants to add
is to run parse analysis on each SQL statement in a plpgsql function.
That seems to me to be a well-defined and useful check, so I think the
scope of the patch is entirely adequate for now.

A bigger issue is that once you think about more than one kind of check,
it becomes apparent that we might need some user-specifiable options to
control which checks are applied. And I see no provision for that here.
This is not something we can add later, at least not without breaking
the API for the check function --- and if we're willing to break API,
why not just add some more parameters to the validator and avoid having
a second function?

On the whole, it might not be a bad idea to have two allowed signatures
for the validator function, rather than inventing an additional column
in pg_language. But the fundamental point IMHO is that there needs to
be a provision to pass language-dependent validation options to the
function, whether it's the existing validator or a separate checker
entry point.

regards, tom lane