The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: not tested Documentation: not tested
Hello It is one of those features that a handful of people would find useful in specific user cases. I think it is a nice to have feature that safeguards your production database against unwanted commits when troubleshooting production problems. Your patch applies fine on master and I am able to run a couple tests on it and it seems to do as described. I noticed that the patch has a per-session variable "default_transaction_committable" that could make all transaction committable or uncommittable on the session even without specifying "begin transaction not committable;" I am wondering if we should have a configurable default at all as I think it should always defaults to true and unchangable. If an user wants a uncommittable transaction, he/she will need to explicitly specify that during "begin". Having another option to change default behavior for all transactions may be a little unsafe, it is possible someone could purposely change this default to false on a production session that needs transactions to absolutely commit, causing damages there. thank you Cary Huang ------------------ Highgo Software Canada www.highgo.ca
