Hello Michaël,
> + * Recursive evaluation of int or double expressions
> + *
> + * Note that currently only integer variables are available, with values
> + * stored as text.
> This comment is incorrect, we only care about integers in this patch.
Indeed!
> Taking patch 1 as a completely independent thing, there is no need to
> introduce PgBenchValueType yet. Similar remark for setIntValue and
> coerceToInt. They are useful afterwards when introducing double types to
> be able to handle double input parameters for the gaussian and other
> functions.
Yes. This is exactly the pain I'm trying to avoid, creating a different
implementation for the first patch, which is just overriden when the
second part is applied...
So I'm trying to compromise, having a several part patch *but* having the
infrastructure ready for the second patch which adds the double type.
Note that the first patch without the second is a loss of time for
everyone, as the nearly only useful functions are the randoms, which
require a double argument, so it does not make sense to apply the first
one if the second one is not to be applied, I think.
> [...]
> (INT64_MIN / -1) should error.
> (INT64_MIN % -1) should result in 0.
> This is missing the division handling.
Oops, indeed I totally messed up when merging the handling of / and %:-(
I have found another issue in the (a) patch: the internal scripts were
using the future random function which do not yet exist, as they are in
patch (b).
Here is a three part v30, which still includes the infrastructure for
future types in the a patch, see my argumentation above.
--
Fabien.
