Re: [v9.3] OAT_POST_ALTER object access hooks

2012/11/19 Alvaro Herrera <alvherre@2ndquadrant.com>:
> Kohei KaiGai wrote:
>> Sorry, I missed the attached version.
>> Please use this revision.
>
> All those direct uses of object_access_hook make me think that the
> InvokeObjectAccessHook() macro we have is insufficient.  Maybe we could
> have InvokeObjectAccessHookArg1() so that instead of
>
> +   if (object_access_hook)
> +   {
> +       ObjectAccessPostCreate  pc_arg;
> +
> +       memset(&pc_arg, 0, sizeof(ObjectAccessPostCreate));
> +       pc_arg.is_internal = is_internal;
> +       (*object_access_hook)(OAT_POST_CREATE, AttrDefaultRelationId,
> +                             RelationGetRelid(rel), attnum, (void *)&pc_arg);
> +   }
>
> we can have
>
>         InvokeObjectAccessHookWithArgs1(OAT_POST_CREATE, AttrDefaultRelationId,
>                                         RelationGetRelid(rel), attnum,
>                                         ObjectAccessPostCreate, is_internal, is_internal)
>
> which would expand into the above.  (Since ObjectAccessPostCreate has
> two members, we presumably need InvokeObjectAccessHookWithArgs2 as
> well.  But that doesn't seem to be used anywhere, so maybe that struct
> member is not necessary?)
>
I'd like to have catalog/objectaccess.c to wrap-up invocation of hooks, rather
than doing all the stuffs with macros. It allows to use local variables, unlike
macros; that has a risk of naming conflict with temporary variable for
ObjectAccessPostCreate.

So, how about to have a following definition for example?

void
InvokePostAlterHookArgs(Oid classId, Oid objectId, int subId,                                        Oid auxiliaryId,
boolis_internal)
 
{   if (object_access_hook)   {       ObjectAccessPostAlter    pa_arg;
       memset(&pa_arg, 0, sizeof(ObjectAccessPostAlter));       pa_arg.auxiliary_id = auxiliary_id;
pa_arg.is_internal= is_internal;       (*object_access_hook)(OAT_POST_ALTER,
classId,objectId, subId,
 
(void *) &pa_arg);   }
}

and

#define InvokePostAlterHook(classId, objectId, subId) \   InvokePostAlterHookArgs(classId, objectId, subId, InvalidOid,
false)

for most points to call.

Even if ObjectAccessPostCreate is extended in the future version, it does not
take changes on most of hook invocation points.
We will be also able to have same semantics on OAT_POST_CREATE and
OAT_DROP as well.

> The second hunk to alter.c does not apply anymore; please rebase.
>
OK,

Thanks,
-- 
KaiGai Kohei <kaigai@kaigai.gr.jp>