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

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to