Re: security hooks on object creation

(2010/11/12 19:34), KaiGai Kohei wrote:
> I revised my patch according to the prior suggestions.
> 
I'm sorry. I revised my patch, but not attached.

Please see this attached one.

Thanks,

> 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>