Re: review: CHECK FUNCTION statement

Hello

2012/2/29 Alvaro Herrera <alvherre(at)commandprompt(dot)com>:
>
> I think the way we're passing down the options to the checker is a bit
> of a mess.  The way it is formulated, it seems to me that we'd need to
> add support code in the core CheckFunction for each option we might want
> to accept in the PL-specific checkers -- including what type of value
> the option receives.  As an example, right now we have all but one
> option taking a string argument (judging from usage of defGetString());
> however, option fatal_errors takes a boolean value, and it converts to
> string "on" or "off" which is supposed to be passed down to the checker.
> This doesn't seem very future-proof.

I don't agree - we has not any datatype that can hold "key/value" list
- and can be used via SQL interface. So two dimensional text array is
simple and generic data type. Theoretically we can use JSON type now,
but I think so array is more generic and better checked then JSON now
(and it require less code - and JSON is still string too). We don't
plan to modify parser to better support of JSON, so text array is more
user friendly. I think so pl checker function can be simply called
with used concept. But I am not against to your proposals. Actually
"concept" of generic options was required in initial discuss and then
there is implemented, but it not used. But cannot be removed, because
probably don't would to change API in next version.

>
> (Also, the patch seems to be passing the fatal_errors value twice: first
> in the options array, where it is ignored by the plpgsql checker, and a
> second time as a separate boolean option.  This needs a cleanup).
>

This is by design. One request for checker function (and check
function statement) was generic support for some optional data. This
can has sense for plperl or plpython, and it are not used now.
Fatal_errors is only proof concept and can be removed. I plan to use
these options in 9.3 for checking of "inline blocks".

> I don't see any good way to pass down generic options in a generic way.
> Maybe we can just state that all option values are going to be passed as
> strings -- is that good enough?  The other option would be to pass them
> using something like pg_node_tree, but then it wouldn't be possible to
> call the checker directly instead of through CHECK FUNCTION, which I
> think was a requirement.  And it'd be a stronger argument against usage
> of SPI to call the checker function from CHECK FUNCTION, but that's an
> unsolved problem.

Using just string needs more complex parsing, and I don't like some
like pg_node_tree too, because it cannot be simple created by hands
for direct call of checker function. Please, accept fact, so we would
to call directly PL checker function - and there all params of this
function should be simple created - and using two dimensional array is
really simple: ARRAY [['source',$$....$$]].

I don't understand why you don't like pass generic options by generic way. Why?

Regards

Pavel Stehule

>
> --
> Álvaro Herrera <alvherre(at)commandprompt(dot)com>
> The PostgreSQL Company - Command Prompt, Inc.
> PostgreSQL Replication, Consulting, Custom Development, 24x7 support