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
