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: PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: extend pgbench expressions with functions
Date: 2016-01-12 08:52:11
Message-ID: alpine.DEB.2.10.1601120951560.26748@sto
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


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.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Etsuro Fujita 2016-01-12 09:00:02 Re: Minor code improvements to create_foreignscan_plan/ExecInitForeignScan
Previous Message Simon Riggs 2016-01-12 08:26:32 Re: Speedup twophase transactions