From: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: poll: CHECK TRIGGER? |
Date: | 2012-03-07 17:28:20 |
Message-ID: | CAFj8pRDsoHwPUOcU0Y3MpqgS5B4=vo0YY3R9YQK81AZC+2bq4Q@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
2012/3/7 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Wed, Mar 7, 2012 at 12:17 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2012-03-07 17:28:37 | Re: Checksums, state of play |
Previous Message | Simon Riggs | 2012-03-07 17:17:21 | Re: a slightly stale comment |