Re: Proposal: SET ROLE hook

From: Joe Conway <mail(at)joeconway(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>
Subject: Re: Proposal: SET ROLE hook
Date: 2016-01-06 18:02:33
Message-ID: 568D56B9.7000304@joeconway.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 01/06/2016 02:39 AM, Pavel Stehule wrote:
> I did a review of this patch.
>
> 1. the proposal is clean and there are not any objection against it. I
> checked a implementation, and it does exactly same what was proposed.
>
> 2. This hook has impact only on SET role to XXX statement, what isn't
> used in our critical path, so there are not any performance impact
>
> 3. I was able to apply patch cleanly without any problems, warnings.
>
> 4. All regress tests was passed
>
> 5. there are not tests and documentation, but it is usual for any hook
>
> I have not any objection
>
> I'll mark this patch as ready for committer

Thanks for the review!

For what it's worth, I did look at Andres' idea and in fact created an
extension available here: https://github.com/pgaudit/set_user

I also looked at implementing equivalent functionality with the
ProcessUtility_hook. That appears feasible but would involve a fair
amount of code duplication from the backend.

Compared to both of these alternatives, I still feel that the specific
SET ROLE hook is cleanest and best path forward. If there are no other
comments or concerns, I will commit this in a day or two.

Thanks,

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Greg Stark 2016-01-06 18:32:48 Re: pglogical_output - a general purpose logical decoding output plugin
Previous Message Tom Lane 2016-01-06 17:54:42 Re: Very confusing installcheck behavior with PGXS