Re: pgbench - doCustom cleanup

On 2018-Nov-17, Fabien COELHO wrote:

> 
> > Attached is a v3, where I have updated inaccurate comments.
> 
> Attached v4 is a rebase after 409231919443984635b7ae9b7e2e261ab984eb1e

Attached v5.  I thought that separating the part that executes the
command was an obvious readability improvement.  Tests still pass,
though maybe I broke something inadvertently.  (I started by thinking
"does this block require a 'fall-through' comment?"  The 600 line
function is pretty hard to read with the multiple messy switches and
conditionals; split into 400 + 200 subroutine it's nicer to reason
about.)

Do we really use the word "move" to talk about state changes?  It sounds
very odd to me.  I would change that to "transition" -- would anybody
object to that?  (Not changed in v5.)

On INSTR_TIME_SET_CURRENT_LAZY(), you cannot just put an "if" inside a
macro -- consider this:
    if (foo)
        INSTR_TIME_SET_CURRENT_LAZY(bar);
    else
        something_else();
Which "if" is the else now attached to?  Now maybe the C standard has an
answer for that (I don't know what it is), but it's hard to read and
likely the compiler will complain anyway.  I wrapped it in "do { }
while(0)" as is customary.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services