Re: extend pgbench expressions with functions

On Wed, Nov 4, 2015 at 4:06 AM, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:
>> 1. I think there should really be two patches here, the first adding
>> functions, and the second adding doubles. Those seem like separate
>> changes. And offhand, the double stuff looks a lot less useful that
>> the function call syntax.
>
> I first submitted the infrastructure part, but I was asked to show how more
> functions could be included, especially random variants. As random
> gaussian/exponential functions require a double argument, there must be some
> support for double values.

Ugh, OK.

>> 2. ddebug and idebug seem like a lousy idea to me.
>
> It was really useful to me for debugging and testing.

That doesn't mean it belongs in the final patch.

>> 3. I'm perplexed by why you've replaced evaluateExpr() with evalInt()
>> and evalDouble().
>
> Basically, the code is significantly shorter and elegant with this option.

I find that pretty hard to swallow. If the backend took this
approach, we've have a separate evaluate function for every datatype,
which would make it completely impossible to support the creation of
new datatypes. And I find this code to be quite difficult to follow.

What I think we should have is a type like PgBenchValue that
represents a value which may be an integer or a double or whatever
else we want to support, but not an expression - specifically a value.
Then when you invoke a function or an operator it gets passed one or
more PgBenchValue objects and can do whatever it wants with those,
storing the result into an output PgBenchValue. So add might look
like this:

void
add(PgBenchValue *result, PgBenchValue *x, PgBenchValue *y)
{
if (x->type == PGBT_INTEGER && y->type == PGBT_INTEGER)
{
result->type = PGBT_INTEGER;
result->u.ival = x->u.ival + y->u.ival;
return;
}
if (x->type == PGBT_DOUBLE && y->type == PGBT_DOUBLE)
{
result->type = PGBT_DOUBLE;
result->u.ival = x->u.dval + y->u.dval;
return;
}
/* cross-type cases, if desired, go here */
/* if no case applies, somehow report an error */
}

The way you have it, the logic for every function and operator exists
in two copies: one in the integer function, and the other in the
double function. As soon as we add another data type - strings,
dates, whatever - we'd need to have cases for all of these things in
those functions as well, and every time we add a function, we have to
update all the case statements for every datatype's evaluation
function. That's just not a scalable model.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company