Re: [v9.2] Object access hooks with arguments support (v1)

On Thu, Oct 13, 2011 at 6:48 AM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>    struct ObjectAccessInfoData {
>        ObjectAccessType   oa_type;
>        ObjectAddress         oa_address;
>        union {
>            struct {
>                HeapTuple       newtuple;
>                TupleDesc       tupdesc;  /* only if create a new relation */
>                    :
>            } post_create;      /* additional info for OAT_POST_CREATE event */
>        }
>    }

That's possibly an improvement over just passing everything opaquely,
but it still doesn't seem very good. I mean, this is C, not Perl or
Python. Anything where you pass a bunch of arguments of indeterminate
type and hope that the person you're calling can figure it out is
inherently pretty dangerous. I had it in mind that the object access
hook mechanism could serve as a simple and generic way of letting an
external module know that an event of a certain type has occurred on a
certain object, and to let that module gain control. But if we have
to pass a raft of arguments in, then it's not generic any more - it's
just a bunch of things that are probably really need to be separate
hooks shoved into a single function.

>> Moreover, if
>> you did document it, I think it would boil down to "this is what
>> sepgsql happens to need", and I don't think that's an acceptable
>> answer. ?We have repeatedly refused to adopt that approach in the
>> past.
>>
> Unfortunately, I still don't have a clear answer for this point.
> However, in general terms, it is impossible to design any interface without
> knowledge of its usage more or less.

Well, that's true. But the hook also has to be maintainable. ISTM
that there's no reasonable way for someone making a modification to
the code to know whether the additional local variable that they just
added to the function should be passed to the hook, or not. Here, at
least as far as I can see, there's no guiding principle that would
enable future contributors to make an intelligent decision about that.
And if someone wanted to write another client for the hook, it's not
very obvious whether the particular things you've chosen to pass here
would be relevant or not. I think if you look over the code you'll
find that there's nowhere else that we have a hook that looks anything
like what you're proposing here.

> At least, this proposition enables modules being informed using
> commonly used data type (such as HeapTuple, TupleDesc), compared
> to the past approach that tried to push all the necessary information
> into argument list individually.

That does seem like a good direction to go in, but you're still
passing a lot of other stuff in there. I guess my feeling is that if
we can't boil down the argument list to a short list of things that
are more-or-less common to all the call sites, we probably need to
rethink the whole design.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company