Re: add modulo (%) operator to pgbench

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
Cc: David Rowley <dgrowleyml(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: add modulo (%) operator to pgbench
Date: 2015-01-05 14:38:39
Message-ID: 20150105143839.GJ1457@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Fabien COELHO wrote:
>
> >I'm also just looking at you ERROR() macro, it seems that core code is
> >quite careful not to use __VA_ARGS__ on compilers where HAVE__VA_ARGS is
> >not defined. I'd say this needs to be fixed too. Have a look at the trick
> >used in elog.h for when HAVE__VA_ARGS is not defined.
>
> Here is a v5 with the vararg macro expanded.

Thanks.

On top of evaluateExpr() we need a comment (generally I think pgbench
could do with more comments; not saying your patch should add them, just
expressing an opinion.) Also, intuitively I would say that the return
values of that function should be reversed: return true if things are
good. Can we cause a stack overflow in this function with a complex
enough expression?

I wonder about LOCATE and LOCATION. Can we do away with the latter, and
keep only LOCATE perhaps with a better name such as PRINT_ERROR_AT or
similar? I would just expand an ad-hoc fprintf in the single place
where the other macro is used.

Are we okay with only integer operands? Is this something we would
expand in the future? Is the gaussian/exp random stuff going to work
with integer operands, if we want to change it to use function syntax,
as expressed elsewhere?

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2015-01-05 14:42:09 Re: add modulo (%) operator to pgbench
Previous Message Alexey Vasiliev 2015-01-05 13:39:28 Re[2]: [HACKERS] Patch: add recovery_timeout option to control timeout of restore_command nonzero status code