From: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> |
---|---|
To: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: PL/pgSQL nested CALL with transactions |
Date: | 2018-03-28 12:54:59 |
Message-ID: | 59d9cd0d-d14f-563f-d87f-ac1954537f3d@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Tomas Vondra | 2018-03-28 13:00:24 | Re: PL/pgSQL nested CALL with transactions |
Previous Message | Pavan Deolasee | 2018-03-28 12:47:55 | Re: [HACKERS] A design for amcheck heapam verification |