Re: review: CHECK FUNCTION statement

Поиск
Список
Период
Сортировка
От Albe Laurenz
Тема Re: review: CHECK FUNCTION statement
Дата
Msg-id D960CB61B694CF459DCFB4B0128514C20742FA96@exadv11.host.magwien.gv.at
обсуждение исходный текст
Ответ на Re: review: CHECK FUNCTION statement  (Pavel Stehule <pavel.stehule@gmail.com>)
Ответы Re: review: CHECK FUNCTION statement  (Greg Smith <greg@2ndQuadrant.com>)
Re: review: CHECK FUNCTION statement  (Pavel Stehule <pavel.stehule@gmail.com>)
Re: review: CHECK FUNCTION statement  (Pavel Stehule <pavel.stehule@gmail.com>)
Список pgsql-hackers
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

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Robert Haas
Дата:
Сообщение: Re: Patch to allow users to kill their own queries
Следующее
От: Robert Haas
Дата:
Сообщение: Re: JSON for PG 9.2