2012/3/7 Robert Haas <robertmh...@gmail.com>: > On Wed, Mar 7, 2012 at 12:17 AM, Pavel Stehule <pavel.steh...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers