Re: pgbench small bug fix

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Aleksander Alekseev <a(dot)alekseev(at)postgrespro(dot)ru>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pgbench small bug fix
Date: 2016-03-03 19:35:29
Message-ID: alpine.DEB.2.10.1603032018170.24239@sto
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


Hello Aleksander,

Thanks for the look at the patch.

>> time pgbench -T 5 -R 0.1 -P 1 -c 2 -j 2
>
> On my laptop this command executes 25 seconds instead of 5.
> I'm pretty sure it IS a bug. Probably a minor one though.

Sure.

> [...] you should probably write:
>
> if(someint > 0)

Ok.

> if(somebool == TRUE)

I like "if (somebool)", the "== TRUE" looks like a tautology, and the
short version is also the current practice in the project.

> Also I suggest to introduce a few new boolean variables with meaningful
> names instead of writing all these long expressions right inside of
> if( ... ).

I agree about the lisibility, but there are semantics issues to consider:

if (short-A && pretty-long-B)

If short-A is false then pretty-long-B is not evaluated, which is a win
because it also costs, I try to order conditions... If I move
pretty-long-B before then the cost is always paid. Now I could write:

if (short-A) {
bool b = pretty-long-B;
if (b) {
...

But this looks contrived and people would raise other questions about such
a strange construct for implementing && in 3 lines, 2 if and 1 variable...

> As a side note I noticed that pgbench.c is not pgindent'ed. Since you
> are modifying this file anyway probably you cold solve this issue too?
> As a separate patch perhaps.

As Robert said, not the purpose of this patch.

Attached is a v3 which test integers more logically. I'm a lazy programmer
who tends to minimize the number of key strokes.

--
Fabien.

Attachment Content-Type Size
pgbench-duration-under-rate-3.patch text/x-diff 1.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2016-03-03 19:37:24 Re: Support for N synchronous standby servers - take 2
Previous Message Alexander Korotkov 2016-03-03 19:27:24 Re: improving GROUP BY estimation