On Thu, Jan 26, 2012 at 7:27 AM, Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
> It seems to me reasonable design.
> The attached patch is rebased one according to your perform-deletion patch.
That looks pretty sensible. But I don't think this is true any more:
+ Please note that it shall not be checked on the objects removed by
+ cascaded deletion according to the standard manner in SQL.
I've been thinking more about the question of object access hooks with
arguments as well. In a couple of designs you've proposed, we've
needed InvokeObjectAccessHook0..11 or whatever, and I don't like that
design - it seems grotty and not type-safe. However, looking at what
you did in this patch, I have another idea. Suppose that we add ONE
additional argument to the object access hook function, as you've done
here, and if a particular type of hook invocation needs multiple
arguments, it can pass them with a struct. In fact, let's use a
struct regardless, for uniformity, and pass the value as a void *.
That is:
typedef struct ObjectAccessDrop { int dropflags;
} ObjectAccessDrop;
At the call site, we do this:
if (object_access_hook)
{ ObjectAccessDrop arg; arg.dropflags = flags; InvokeObjectAccessHook(..., arg);
}
If there's no argument, then we can just do:
InvokeObjectAccessHook(..., NULL);
The advantage of this is that if we change the structure definition,
loadable modules compiled against a newer code base should either (1)
still work or (2) fail to compile. The way we have it right now, if
we decide to pass a second argument (say, the DropBehavior) to the
hook, we're potentially going to silently break sepgsql no matter how
we do it. But if we enforce use of a struct, then the only thing the
client should ever be doing with the argument it gets is casting it to
ObjectAccessDrop *. Then, if we've added fields to the struct, the
code will still do the right thing even if the field order has been
changed or whatever. If we've removed fields or changed their data
types, things should blow up fairly obviously instead of seeming to
work but actually failing.
Thoughts?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
