2011/12/16 Albe Laurenz <laurenz.a...@wien.gv.at>: > Pavel Stehule wrote: >> one small update - better emulation of environment for security >> definer functions > > Patch applies and compiles fine, core functionality works fine. > > I found a little bug: > > In backend/commands/functioncmds.c, > function CheckFunction(CheckFunctionStmt *stmt), > while you perform the table scan for CHECK FUNCTION ALL, > you use the variable funcOid to hold the OID of the current > function in the loop. > > If no appropriate function is found in the loop, the > check immediately after the table scan will not succeed > because funcOid holds the OID of the last function scanned > in the loop. > As a consequence, CheckFunctionById is called for this > function. > > Here is a demonstration: > test=> CHECK FUNCTION ALL IN SCHEMA pg_catalog; > [...] > NOTICE: skip check function "plpgsql_checker(oid,regclass,text[])", uses C > language > NOTICE: skip check function "plperl_call_handler()", uses C language > NOTICE: skip check function "plperl_inline_handler(internal)", uses C > language > NOTICE: skip check function "plperl_validator(oid)", uses C language > NOTICE: skip check function "plperl_validator(oid)", language "c" hasn't > checker function > CHECK FUNCTION > > when it should be: > test=> CHECK FUNCTION ALL IN SCHEMA pg_catalog; > [...] > NOTICE: skip check function "plpgsql_checker(oid,regclass,text[])", uses C > language > NOTICE: skip check function "plperl_call_handler()", uses C language > NOTICE: skip check function "plperl_inline_handler(internal)", uses C > language > NOTICE: skip check function "plperl_validator(oid)", uses C language > NOTICE: nothing to check > CHECK FUNCTION
I'll fix it > > > Another thing struck me as odd: > > You have the option "fatal_errors" for the checker function, but you > special case it in CheckFunction(CheckFunctionStmt *stmt) and turn > errors to warnings if it is not set. > > Wouldn't it be better to have the checker function ereport a WARNING > or an ERROR depending on the setting? Options should be handled by the > checker function. The behave that I use, is more rubust and there is only a few lines of code more. a) It ensure expectable behave for third party checker function - exception in checker function doesn't break a multi statement check function b) It can ensure same format of error message - because it is transformed on top level Regards Pavel > > Yours, > Laurenz Albe -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers