On 3/27/18 20:43, Tomas Vondra wrote: >>> 3) utility.c >>> >>> I find this condition rather confusing: >>> >>> (!(context == PROCESS_UTILITY_TOPLEVEL || >>> context == PROCESS_UTILITY_QUERY_NONATOMIC) || >>> IsTransactionBlock()) >>> >>> I mean, negated || with another || - it took me a while to parse what >>> that means. I suggest doing this instead: >>> >>> #define ProcessUtilityIsAtomic(context) \ >>> (!(context == PROCESS_UTILITY_TOPLEVEL || >>> context == PROCESS_UTILITY_QUERY_NONATOMIC)) >>> >>> (ProcessUtilityIsAtomic(context) || IsTransactionBlock()) >> fixed >> > Ummm, I still see the original code here.
I put the formula into a separate variable isAtomicContext instead of repeating it twice. I think that makes it clearer. I'm not sure splitting it up like above makes it better, because the IsTransactionBlock() is part of the "is atomic". Maybe adding a comment would make it clearer. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
