Thread: Trigger functions don't obey "strict" setting?

I just read across the code in command/trigger.c:ExecCallTriggerFunc() and
apparently the trigger function is called unconditionally even if the
"strict" flag is set. Perhaps this should be amended somewhere.

For coding clarity and convenience I'd suggest that we add another
function as a wrapper around FunctionCallInvoke() which does the right
thing with "strict". We could call that FunctionCallInvoke(), and call the
current version FunctionCallInvokeNoNulls() or some such.

Btw., FunctionCallInvoke() would look to be the most prominent place to
hook in the "setuid" feature. For that purpose I'd make the macro an
inline function instead.

-- 
Peter Eisentraut      peter_e@gmx.net       http://yi.org/peter-e/

Peter Eisentraut <peter_e@gmx.net> writes:
> I just read across the code in command/trigger.c:ExecCallTriggerFunc() and
> apparently the trigger function is called unconditionally even if the
> "strict" flag is set. Perhaps this should be amended somewhere.

Since triggers take no parameters, I'm not sure this is wrong.  But if
it is wrong, then the code at fault is ExecCallTriggerFunc, not
FunctionCallInvoke.  FunctionCallInvoke is just for *invoking*, not
for deciding whether to invoke.

> Btw., FunctionCallInvoke() would look to be the most prominent place to
> hook in the "setuid" feature. For that purpose I'd make the macro an
> inline function instead.

Ugh.  The performance cost would be excessive.  Instead, when fmgr is
setting up to call a setuid function, have it insert an extra level of
function handler that does the save/setup/restore of current UID.
That way, the cost is zero when you're not using the feature (which is
nearly always).
        regards, tom lane

Tom Lane writes:

> > Btw., FunctionCallInvoke() would look to be the most prominent place to
> > hook in the "setuid" feature. For that purpose I'd make the macro an
> > inline function instead.
> 
> Ugh.  The performance cost would be excessive.

In the path of a "normal" function call is only one extra `if (bool)'
statement. There are certainly more "excessive" performance problems than
that, no?

> Instead, when fmgr is setting up to call a setuid function, have it
> insert an extra level of function handler that does the
> save/setup/restore of current UID.

I don't quite understand. Do you mean like a PL function handler? But then
this thing wouldn't work for external PL's unless we either have a setuid
version of each or have nested handlers.


-- 
Peter Eisentraut      peter_e@gmx.net       http://yi.org/peter-e/

Where were we on this? Yes/No/Maybe?

Peter Eisentraut writes:

> Tom Lane writes:
> 
> > > Btw., FunctionCallInvoke() would look to be the most prominent place to
> > > hook in the "setuid" feature. For that purpose I'd make the macro an
> > > inline function instead.
> > 
> > Ugh.  The performance cost would be excessive.
> 
> In the path of a "normal" function call is only one extra `if (bool)'
> statement. There are certainly more "excessive" performance problems than
> that, no?
> 
> > Instead, when fmgr is setting up to call a setuid function, have it
> > insert an extra level of function handler that does the
> > save/setup/restore of current UID.
> 
> I don't quite understand. Do you mean like a PL function handler? But then
> this thing wouldn't work for external PL's unless we either have a setuid
> version of each or have nested handlers.

-- 
Peter Eisentraut      peter_e@gmx.net       http://yi.org/peter-e/

Peter Eisentraut <peter_e@gmx.net> writes:
> Where were we on this? Yes/No/Maybe?
>> 
>>>> Instead, when fmgr is setting up to call a setuid function, have it
>>>> insert an extra level of function handler that does the
>>>> save/setup/restore of current UID.
>> 
>> I don't quite understand. Do you mean like a PL function handler? But then
>> this thing wouldn't work for external PL's unless we either have a setuid
>> version of each or have nested handlers.

Sorry, I forgot to reply.  Nested handlers were exactly what I was
advocating.  Or more accurately, *a* nested handler; you'd only need
one regardless of the target function's language.  I'm envisioning
that it'd have fn_extra pointing at a block that contains the UID to
be used for the call and the FmgrInfo for the underlying function.
        regards, tom lane