Re: Patch proposal: New hooks in the connection path

On Fri, Jul 01, 2022 at 09:48:40AM +0200, Drouvot, Bertrand wrote:
>> However, I'm personally not okay with having multiple hooks
>> as proposed in the v1 patch.
>
> I agree that it would be great to reduce the number of proposed hooks.
>
> But,
>
>> Can we think of having a single hook
>
> The proposed hooks are triggered during errors (means that the connection
> attempt break) and:
>
> - In the connection paths that will not reach the ClientAuthentication_hook
> at all: those are the ones related to the bad startup packet and timeout
> while processing the startup packet.
>
> or
>
> - After the ClientAuthentication_hook is fired: those are the bad db oid,
> bad db name and bad perm ones.
>
> So, It does look like having only one hook would require refactoring in the
> connection path and I'm not sure if this is worth it.
>
>> or
>> enhancing the existing ClientAuthentication_hook where we pass a
>> PURPOSE parameter (CONN_SUCCESS, CONN_FAILURE, CONN_FOO, CONN_BAR
>> ....) tp the hook?
>
> I think one could already "predict" the bad db and bad perm errors within
> the current ClientAuthentication_hook.
>
> But in case of multiple "possible" errors (within the same connection
> attempt) how could we know for sure the one that will be actually reported?
> That's why i think the best way is to put new hooks as close as possible to
> the place where the related errors are reported.
>
> What do you think?

Could we model this after fmgr_hook? The first argument in that hook
indicates where it is being called from. This doesn't alleviate the need
for several calls to the hook in the authentication logic, but extension
authors would only need to define one hook.

That being said, I don't see why this information couldn't be provided in a
system view. IMO it is generically useful.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com