The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, passed
Spec compliant:           tested, passed
Documentation:            tested, passed

Hi Daniel,

I have reviewed the patch and I liked it (well I did liked it already since it 
was a part of login trigger patch previously). 
All tests are passed and the manual experiments with all types of event 
triggers are also passed.

Everything is fine and I think It can be marked as Ready for Committer, 
although I have one final question.
There is a complete framework for disabling various types of the event triggers 
separately, but, the list of valid GUC values only include 'all' and 'none'. 
Why not adding support for all the event trigger types separately? Everything 
is already there in the patch; the only thing needed is expanding couple of 
enums. It's cheap in terms of code size and even cheaper in terms of 
performance. And moreover - it would be a good example for anyone adding new 
trigger types.

The new status of this patch is: Waiting on Author

Reply via email to