Thanks for your reviewing, and sorry for the late reply.
I've not been available for a few days.
(2010/11/22 12:11), Robert Haas wrote:
> 2010/11/12 KaiGai Kohei<kaigai@ak.jp.nec.com>:
>> (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.
>
> I'm satisfied with this approach, although I intend to change
> InvokeObjectAccessHook0 to simply InvokeObjectAccessHook before
> committing it;
OK. We have no other object-access-type which takes any arguments
right now. It is quite cosmetic things, so we may be able to add
the number of arguments later, such as SysCache.
> and correct your use of AttributeRelationId to
> RelationRelationId for consistency with the rest of the code.
Oops, it was my bug. I'll fix it.
> What
> I'm not quite sure about is where to put the definitions you've added
> to a new file utils/hooks.h; I don't feel that's a very appropriate
> location. It's tempting to put them in utils/acl.h just because this
> is vaguely access-control related and that header is already included
> in most of the right places, but maybe that's too much of a stretch;
> or perhaps catalog/catalog.h, although that doesn't feel quite right
> either. If we are going to add a new header file, I still don't like
> utils/hooks.h much - it's considerably more generic than can be
> justified by its contents.
>
I don't think utils/acl.h is long-standing right place, because we
intended not to restrict the purpose of this hooks to access controls
as you mentioned.
I think somewhere under the catalog/ directory is a good idea because
it hooks events that user wants (eventually) to modify system catalogs.
How about catalog/hooks.h, instead of utils/hooks.h?
Thanks,
--
KaiGai Kohei <kaigai@ak.jp.nec.com>