Hello Michaël,
> 1) When precising a negative parameter in the gaussian and exponential
> functions an assertion is triggered:
> Assertion failed: (parameter > 0.0), function getExponentialRand, file
> pgbench.c, line 494.
> Abort trap: 6 (core dumped)
> An explicit error is better.
Ok for assert -> error.
> 2) When precising a value between 0 and 2, pgbench will loop
> infinitely. For example:
> \set cid debug(random_gaussian(-100, 100, 0))
> In both cases we just lack sanity checks with PGBENCH_RANDOM,
> PGBENCH_RANDOM_EXPONENTIAL and PGBENCH_RANDOM_GAUSSIAN. I think that
> those checks would be better if moved into getExponentialRand & co
> with the assertions removed. I would personally have those functions
> return a boolean status and have the result in a pointer as a function
> argument.
ISTM that if pgbench is to be stopped, the simplest option is just to
abort with a nicer error message from the get*Rand function, there is no
need to change the function signature and transfer the error management
upwards.
> Another thing itching me is that ENODE_OPERATOR is actually useless
> now that there is a proper function layer. Removing it and replacing
> it with a set of functions would be more adapted and make the code
> simpler, at the cost of more functions and changing the parser so as
> an operator found is transformed into a function expression.
Hmmm. Why not, although the operator management is slightly more efficient
(eg always 2 operands...).
> I am attaching a patch where I tweaked a bit the docs and added some
> comments for clarity. Patch is switched to "waiting on author" for the
> time being.
Ok. I'm hesitating about removing the operator management, especially if
I'm told to put it back afterwards.
--
Fabien.
