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
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.
Yours,
Laurenz Albe
