2010/11/17 KaiGai Kohei <kai...@ak.jp.nec.com>: > I revised my patch as I attached. > > The hook function is modified and consolidated as follows: > > typedef enum FunctionCallEventType > { > FCET_BE_HOOKED, > FCET_PREPARE, > FCET_START, > FCET_END, > FCET_ABORT, > } FunctionCallEventType; > > typedef Datum (*function_call_event_type)(Oid functionId, > FunctionCallEventType event, > Datum event_arg); > extern PGDLLIMPORT function_call_event_type function_call_event_hook; > > Unlike the subject of this e-mail, now it does not focus on only switching > security labels during execution of a certain functions. > For example, we may use this hook to track certain functions for security > auditing, performance tuning, and others. > > In the case of SE-PgSQL, it shall return BoolGetDatum(true), if the target > function is configured as a trusted procedure, then, this invocation will > be hooked by fmgr_security_definer. In the first call, it shall compute > the security context to be assigned during execution on FCET_PREPARE event. > Then, it switches to the computed label on the FCET_START event, and > restore it on the FCET_END or ECET_ABORT event.
This seems like it's a lot simpler than before, which is good. It looks to me as though there should really be two separate hooks, though, one for what is now FCET_BE_HOOKED and one for everything else. For FCET_BE_HOOKED, you want a function that takes an Oid and returns a bool. For the other event types, the functionId and event arguments are OK, but I think you should forget about the save_datum stuff and just always pass fcache->flinfo and &fcache->private. The plugin can get the effect of save_datum by passing around whatever state it needs to hold on to using fcache->private. So: bool (*needs_function_call_hook)(Oid fn_oid); void (*function_call_hook)(Oid fn_oid, FunctionCallEventType event, FmgrInfo flinfo, Datum *private); Another general comment is that you've not done a very complete job updating the comments; there are several of them in fmgr.c that are no longer accurate. Also, please zap the unnecessary whitespace changes. Thanks, -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers