Re: security hooks on object creation - Mailing list pgsql-hackers

I revised my patch according to the prior suggestions.

Invocation of the hooks is encapsulated within macro, not function:
 + #define InvokeObjectAccessHook0(access,classId,objectId,subId)        \ +   do {
                          \ +       if (object_access_hook)                                         \ +
(*object_access_hook)((access),(classId),(objectId),(subId));\ +   } while(0)
 

The "0" of tail means that it does not takes any arguments except for
object-ids, like syscache code.
It will reduce impact when we want to add arguments of the hooks.


In the previous version, it just support seven object classes that is
allowed to assign security labels. But, Robert pointed out the purpose
of post-creation hook is limited to security labeling. So, I expand its
coverage into all the commentable object classes.
- relations:    heap_create_with_catalog()- constraint:    CreateConstraintEntry()- conversion:    ConversionCreate()-
schema:   NamespaceCreate()- operator:    OperatorCreate() and OperatorShellMake()- procedure:    ProcedureCreate()-
type:   TypeCreate() and TypeShellMake()- database:    createdb()- cast:    CreateCast()- opfamily:
CreateOpFamily()-opclass:    DefineOpClass()- language:    create_proc_lang()- attribute:    ATExecAddColumn()-
tablespace:   CreateTableSpace()- trigger:    CreateTrigger()- ts_parser:    DefineTSParser()- ts_dict:
DefineTSDictionary()-ts_template:    DefineTSTemplate()- ts_config:    DefineTSConfiguration()- role:    CreateRole()-
rule:   InsertRule()- largeobject:    inv_create()
 

The post-creation hooks are put on the place just after adding dependency
of the new object, if the object class uses dependency mechanism.
I believe it will be a clear guidance for the future maintenance works.

Thanks,

(2010/11/11 7:41), KaiGai Kohei wrote:
> (2010/11/11 3:00), Robert Haas wrote:
>> On Wed, Nov 10, 2010 at 8:33 AM, KaiGai Kohei<kaigai@kaigai.gr.jp> wrote:
>>> (2010/11/10 13:06), Robert Haas wrote:
>>>>>
>>>>> In this patch, we put InvokeObjectAccessHook0 on the following functions.
>>>>>
>>>>> - heap_create_with_catalog() for relations/attributes
>>>>> - ATExecAddColumn() for attributes
>>>>> - NamespaceCreate() for schemas
>>>>> - ProcedureCreate() for aggregates/functions
>>>>> - TypeCreate() and TypeShellMake() for types
>>>>> - create_proc_lang() for procedural languages
>>>>> - inv_create() for large objects
>>>>
>>>> I think you ought to try to arrange to avoid the overhead of a
>>>> function call in the common case where nobody's using the hook.
>>>> That's why I originally suggested making InvokeObjectAccessHook() a
>>>> macro around the actual function call.
>>>>
>>> Hmm. Although I have little preference here, the penalty to call
>>> an empty function (when no plugins are installed) is not visible,
>>> because frequency of DDL commands are not high.
>>> Even so, is it necessary to replace them by macros?
>>
>> It's a fair point. I'm open to other opinions but my vote is to shove
>> a macro in there. A pointer test is cheaper than a function call, and
>> doesn't really complicate things much.
>>
> Since I have no strong preference function call here, so, I'll revise my
> patch according to your vote.
> 
> Thanks,


-- 
KaiGai Kohei <kaigai@ak.jp.nec.com>