Re: poll: CHECK TRIGGER? - Mailing list pgsql-hackers

2012/3/7 Robert Haas <robertmhaas@gmail.com>:
> On Wed, Mar 7, 2012 at 12:17 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> Robert, please, can you comment to this issue? And other, please. I am
>> able to fix syntax to any form where we will have agreement.
>
> Well, so far I don't see that anyone has offered a compelling reason
> why this needs core syntax support.  If we just define a function
> called plpgsql_checker() or plpgsql_lint() or whatever, someone can
> check one function like this:
>
> SELECT plpgsql_checker('myfunc(int)'::regprocedure);
>
> If they want to check all the functions in a schema, they can do this:
>
> SELECT plpgsql_checker(oid) FROM pg_proc WHERE prolang = (SELECT oid
> FROM pg_language WHERE lanname = 'plpgsql') AND pronamespace = (SELECT
> oid FROM pg_namespace WHERE nspname =  'myschema');
>
> If they want to check all functions they own, they can do this:
>
> SELECT plpgsql_checker(oid) FROM pg_proc WHERE prolang = (SELECT oid
> FROM pg_language WHERE lanname = 'plpgsql') AND pronamespace = (SELECT
> oid FROM pg_authid WHERE rolname =  'myrole');
>
> Any other combination of things that they want to check can be
> accommodated by varying the WHERE clause.  If we think there are
> common cases like the above that we want to make easy, we can do that
> by creating wrapper functions called, e.g.
> plpgsql_check_namespace(text) and plpgsql_check_role(text) that are
> just SQL functions that execute the query mentioned above.  If we find
> that the convenience functions we've added don't cover all the cases
> we care about, it's a lot easier and less invasive to just add a few
> more than it is to invent new syntax.  Moreover, it doesn't require a
> major release cycle: using functional notation, a customer who wants
> to check all security definer functions owned by roles that are
> members of the dev group can do it just by adjusting the queries shown
> above.  If they want an extra convenience function for that, they can
> easily define one.  Even with dedicated syntax it's still possible to
> define convenience functions by wrapping the CHECK FUNCTION statement
> up in a user-defined function that dynamically generates and executes
> SQL and then calling it repeatedly from a query, but that's more work.
>
> As things stand today, the "checker" infrastructure is a very large
> percentage of this patch.  Ripping all that out and just exposing this
> as a function, or a couple of functions, will make the patch much
> smaller and simpler, and allow us to focus on what I think we really
> should be worrying about here, which is the quality and value of the
> tests being performed.  I don't necessarily say that it could *never*
> make sense to add any syntax support here, but I do think there's no
> real clear need for it and that, inasmuch as this is a new feature, it
> doesn't really make sense for us to commit ourselves to more than
> necessary.  tsearch lived without core support for several releases
> until it became clear that it was a sufficiently important feature to
> merit tighter integration.  Furthermore, the functional syntax being
> proposed here is exactly the same kind of syntax that we use for many
> other things that are arguably far more important.  If
> pg_start_backup() isn't important enough to be implemented as an SQL
> command rather than a function, then I don't think this is either.
>
> Just to be clear, I am not proposing that we get rid of CHECK TRIGGER
> and keep CHECK FUNCTION.  I'm proposing that we get rid of all of the
> dedicated syntax support, and expose it all through one or more
> SQL-callable functions.  If we need both
> plpgsql_check_function(procoid) and plpgsql_check_trigger(tgoid), no
> problem.

this is very near to my original design. This design is general so we
really don't special statements - because any action can be processed
in combination query to pg_catalog and calling checker function.

The most expected value of special statements is simplicity for usage
- checking function is common task - it should be called significantly
more often than pg_start_backup() or some tsearch maintaining
procedures.

just: "check function name()" is more shorter than "select
plpgsql_check_function('name()')" and what is important - it is
supported by autocomplete in psql. Other arguments is possibility to
show result.

A special statement has higher possibility to put output more clean
and readable - I am not able do it in function.

I agree, so this patch is relative long, but almost all code in these
statement is related to multiple checking. When I remove it (or when I
simplify)  - I can significantly reduce patch (or this part)

But still I think so some reduced CHECK statements is good idea

mainly for:

* autocomplete support
* more readable output in terminal

I don't know if this is enough

Regards

Pavel







>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company