Re: extend pgbench expressions with functions

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: extend pgbench expressions with functions
Date: 2016-02-15 09:58:22
Message-ID: alpine.DEB.2.10.1602150929210.18755@sto
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


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.

Attachment Content-Type Size
pgbench-funcs-30-a.patch text/x-diff 21.8 KB
pgbench-funcs-30-b.patch text/x-diff 27.4 KB
pgbench-funcs-30-c.patch text/x-diff 8.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dmitry Ivanov 2016-02-15 10:54:41 [WIP] ALTER ... OWNER TO ... CASCADE
Previous Message Etsuro Fujita 2016-02-15 09:35:07 Re: Optimization for updating foreign tables in Postgres FDW